MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Frazer Clement Date:July 29 2009 10:50am
Subject:Re: bzr commit into mysql-5.4 branch (alik:2804) Bug#45584
View as plain text  
One further comment from testing locally :

Alexander Nozdrin wrote:
> +
> +/************************************************************************/
> +
> +/*
> +  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
> +
> +/************************************************************************/
> +
>
>   
...

> +
> +bool ip_to_hostname(struct sockaddr_storage *ip_storage,
> +                    char **hostname, uint *connect_errors)
> +{
>   
...
> +
> +  char hostname_buffer[NI_MAXHOST];
> +
> +  DBUG_PRINT("info", ("Resolving '%s'...", (const char *) ip_string));
> +
> +  err_code= getnameinfo(ip, sizeof (sockaddr_storage),
> +                        hostname_buffer, NI_MAXHOST, NULL, 0, NI_NAMEREQD);
> +
> +  if (err_code == MY_NONAME_ERR_CODE)
>   
This should be
if (err_code == EAI_NONAME)
as that is what is returned when there is no name mapping for an 
address. 
http://www.opengroup.org/onlinepubs/009695399/functions/getnameinfo.html
The MY_NONAME_ERR_CODE workaround looks like it's intended for the other 
mapping (address from name using getaddrinfo), and is still used below 
in the Fcrdns code.
This is highlighted when attempting to connect from an IP with no host 
record.  The err_code is not EAI_NODATA, so we hit the else branch here 
and fail to get to checking the ACL.

> +  {
> +    /*
> +      There is no reverse address mapping for the IP address. A host name
> +      can not be resolved.
> +    */
> +
> +    DBUG_PRINT("error", ("IP address '%s' could not be resolved: "
> +                         "no reverse address mapping.",
> +                         (const char *) ip_string));
> +
> +    sql_print_warning("IP address '%s' could not be resolved: "
> +                      "no reverse address mapping.",
> +                      (const char *) ip_string);
> +
> +    bool err_status= add_hostname(ip_string, NULL);
> +
> +    *hostname= NULL;
> +    *connect_errors= 0; /* New IP added to the cache. */
> +
> +    DBUG_RETURN(err_status);
> +  }
> +  else if (err_code)
>    {
> -    DBUG_PRINT("error",("out of memory"));
> -    DBUG_RETURN(0);
> +    DBUG_PRINT("error", ("IP address '%s' could not be resolved: "
> +                         "getnameinfo() returned %d.",
> +                         (const char *) ip_string,
> +                         (int) err_code));
> +
> +    sql_print_warning("IP address '%s' could not be resolved: "
> +                      "getnameinfo() returned error (code: %d).",
> +                      (const char *) ip_string,
> +                      (int) err_code);
> +
> +    DBUG_RETURN(TRUE);
>    }
>  
> -  /* Don't accept hostnames that starts with digits because they may be
> -    false ip:s */
> -  if (my_isdigit(&my_charset_latin1, name[0]))
> +  DBUG_PRINT("info", ("IP '%s' resolved to '%s'.",
> +                      (const char *) ip_string,
> +                      (const char *) hostname_buffer));
> +
> +  /*
> +    Validate hostname: the server does not accept host names, which
> +    resemble IP addresses.
> +
> +    The thing is that theoretically, a host name can be in a form of IPv4
> +    address (123.example.org, or 1.2 or even 1.2.3.4). We have to deny such
> +    host names because ACL-systems is not designed to work with them.
> +
> +    For exmaple, it is possible to specify a host name mask (like
> +    192.168.1.%) for an ACL rule. Then, if IPv4-like hostnames are allowed,
> +    there is a security hole: instead of allowing access for
> +    192.168.1.0/255 network (which was assumed by the user), the access
> +    will be allowed for host names like 192.168.1.example.org.
> +  */
> +
> +  if (!is_hostname_valid(hostname_buffer))
>    {
> -    char *pos;
> -    for (pos= name+1 ; my_isdigit(&my_charset_latin1, *pos); pos++) ;
> -    if (*pos == '.')
> -    {
> -      DBUG_PRINT("error",("mysqld doesn't accept hostnames that starts with a number
> followed by a '.'"));
> -      goto add_wrong_ip_and_return;
> -    }
> +    DBUG_PRINT("error", ("IP address '%s' has been resolved "
> +                         "to the host name '%s', which resembles "
> +                         "IPv4-address itself.",
> +                         (const char *) ip_string,
> +                         (const char *) hostname_buffer));
> +
> +    sql_print_warning("IP address '%s' has been resolved "
> +                      "to the host name '%s', which resembles "
> +                      "IPv4-address itself.",
> +                      (const char *) ip_string,
> +                      (const char *) hostname_buffer);
> +
> +    bool err_status= add_hostname(ip_string, NULL);
> +
> +    *hostname= NULL;
> +    *connect_errors= 0; /* New IP added to the cache. */
> +
> +    DBUG_RETURN(err_status);
>    }
>  
> -  bzero(&hints, sizeof (struct addrinfo));
> +  /* Get IP-addresses for the resolved host name (FCrDNS technique). */
> +
> +  struct addrinfo hints;
> +  struct addrinfo *addr_info_list;
> +
> +  memset(&hints, 0, sizeof (struct addrinfo));
>    hints.ai_flags= AI_PASSIVE;
> -  hints.ai_socktype= SOCK_STREAM;  
> +  hints.ai_socktype= SOCK_STREAM;
>    hints.ai_family= AF_UNSPEC;
>  
> -  gxi_error= getaddrinfo(hostname_buff, NULL, &hints, &res_lst);
> -  if (gxi_error)
> +  DBUG_PRINT("info", ("Getting IP addresses for hostname '%s'...",
> +                      (const char *) hostname_buffer));
> +
> +  err_code= getaddrinfo(hostname_buffer, NULL, &hints, &addr_info_list);
> +
> +  if (err_code == MY_NONAME_ERR_CODE)
>    {
>      /*
> -      Don't cache responses when the DSN server is down, as otherwise
> +      Don't cache responses when the DNS server is down, as otherwise
>        transient DNS failure may leave any number of clients (those
>        that attempted to connect during the outage) unable to connect
>        indefinitely.
>      */
> -    /* 
> -      When this code was written there were issues with winsock in pusbuild, 
> -      this define is in this place for this reason.
> -    */
> -    DBUG_PRINT("error",("getaddrinfo returned %d", gxi_error));
> -#ifdef EAI_NODATA
> -    if (gxi_error == EAI_NODATA )
> -#else
> -    if (gxi_error == EAI_NONAME )
> -#endif
> -      add_wrong_ip(in);
>  
> -    my_free(name,MYF(0));
> -    DBUG_RETURN(0);
> +    bool err_status= add_hostname(ip_string, NULL);
> +
> +    *hostname= NULL;
> +    *connect_errors= 0; /* New IP added to the cache. */
> +
> +    DBUG_RETURN(err_status);
> +  }
> +  else if (err_code)
> +  {
> +    DBUG_PRINT("error", ("getaddrinfo() failed with error code %d.", err_code));
> +    DBUG_RETURN(TRUE);
>    }
>  
> -  /* Check that 'getaddrinfo' returned the used ip */
> -  for (t_res= res_lst; t_res; t_res=t_res->ai_next)
> +  /* Check that getaddrinfo() returned the used IP (FCrDNS technique). */
> +
> +  DBUG_PRINT("info", ("The following IP addresses found for '%s':",
> +                      (const char *) hostname_buffer));
> +
> +  for (struct addrinfo *addr_info= addr_info_list;
> +       addr_info; addr_info= addr_info->ai_next)
> +
>    {
> -    if (!memcmp(&(t_res->ai_addr), in,
> -                sizeof(struct sockaddr_storage) ) )
> +    struct sockaddr *resolved_ip= addr_info->ai_addr;
> +    char resolved_ip_key[HOST_ENTRY_KEY_SIZE];
> +
>      {
> -      add_hostname(in, name);
> -      freeaddrinfo(res_lst);
> -      DBUG_RETURN(name);
> +      bool err_status= hostname_cache_get_key(resolved_ip, resolved_ip_key);
> +      DBUG_ASSERT(!err_status);
>      }
> +
> +    DBUG_PRINT("info", ("  - '%s'", (const char *) resolved_ip_key));
> +
> +    if (strcmp(ip_string, resolved_ip_key) == 0)
> +    {
> +      /* Copy host name string to be stored in the cache. */
> +
> +      *hostname= my_strdup(hostname_buffer, MYF(0));
> +
> +      if (!*hostname)
> +      {
> +        DBUG_PRINT("error", ("Out of memory."));
> +
> +        freeaddrinfo(addr_info_list);
> +        DBUG_RETURN(TRUE);
> +      }
> +
> +      break;
> +    }
> +  }
> +
> +  /* Log resolved IP-addresses if no match was found. */
> +
> +  if (!*hostname)
> +  {
> +    sql_print_information("Hostname '%s' does not resolve to '%s'.",
> +                          (const char *) hostname_buffer,
> +                          (const char *) ip_string);
> +    sql_print_information("Hostname '%s' has the following IP addresses:",
> +                          (const char *) hostname_buffer);
> +
> +    for (struct addrinfo *addr_info= addr_info_list;
> +         addr_info; addr_info= addr_info->ai_next)
> +
> +    {
> +      struct sockaddr *resolved_ip= addr_info->ai_addr;
> +      char resolved_ip_key[HOST_ENTRY_KEY_SIZE];
> +
> +      hostname_cache_get_key(resolved_ip, resolved_ip_key);
> +
> +      sql_print_information(" - %s\n", (const char *) resolved_ip_key);
> +    }
> +  }
> +
> +  /* Free the result of getaddrinfo(). */
> +
> +  freeaddrinfo(addr_info_list);
> +
> +  /* Add an entry for the IP to the cache. */
> +
> +  bool err_status;
> +
> +  if (*hostname)
> +  {
> +    err_status= add_hostname(ip_string, *hostname);
> +    *connect_errors= 0;
> +  }
> +  else
> +  {
> +    DBUG_PRINT("error",("Couldn't verify hostname with getaddrinfo()."));
> +
> +    err_status= add_hostname(ip_string, NULL);
> +    *hostname= NULL;
> +    *connect_errors= 0;
>    }
> -  freeaddrinfo(res_lst);
> -  DBUG_PRINT("error",("Couldn't verify hostname with getaddrinfo"));
>  
> -add_wrong_ip_and_return:
> -  my_free(name,MYF(0));
> -  add_wrong_ip(in);
> -  DBUG_RETURN(0);
> +  DBUG_RETURN(err_status);
>  }
>   

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