[Spice-devel] [common PATCH 3/3 v3] ssl-verify: Changed IPv4 hostname to IPv6

[Fabiano Fidêncio]

On Wed, Oct 14, 2015 at 4:57 PM, Lukas Venhoda <lvenhoda at redhat.com> wrote:
> Change inet_aton function to glib functions.
>
> inet_aton only supported IPv4 addresses, and wasn't available on windows
> machines. GInetAddress functions support IPv6 natively, and requires less
> boilerplate code then IPv6 gettaddrinfo().
> ---
> Changes since v2:
>  - Now uses glib gio GInetAddress
>     - Removes compatibility issue with WindowsXP
>     - No need for union
>     - No need for ip_address_to_string() function
>  - Squashed commits
>     - Split not needed anymore
>     - Not as much code with glib
>
> Changes since v1:
>  - Changed inet_pton() to getaddrinfo()
>  - Removed inet_aton() compatibility function
>  - Changed 2 variables into a union
>  - Cleaned up memcmp condition
>   - Added inet_ntop() compatibility function
>     - For Windows XP
>  - Added ip_address_to_string() function
>  - Cleaned up alt match debug message
> ---
>  common/ssl_verify.c | 45 +++++++++++++++++++++------------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/common/ssl_verify.c b/common/ssl_verify.c
> index fe04409..25c3455 100644
> --- a/common/ssl_verify.c
> +++ b/common/ssl_verify.c
> @@ -31,19 +31,7 @@
>  #endif
>  #include <ctype.h>
>  #include <string.h>
> -
> -#ifdef WIN32
> -static int inet_aton(const char* ip, struct in_addr* in_addr)
> -{
> -    unsigned long addr = inet_addr(ip);
> -
> -    if (addr == INADDR_NONE) {
> -        return 0;
> -    }
> -    in_addr->S_un.S_addr = addr;
> -    return 1;
> -}
> -#endif
> +#include <gio/gio.h>
>
>  static int verify_pubkey(X509* cert, const char *key, size_t key_size)
>  {
> @@ -202,20 +190,29 @@ static int verify_hostname(X509* cert, const char *hostname)
>                      return 1;
>                  }
>              } else if (name->type == GEN_IPADD) {
> -                struct in_addr addr;
> -                int addr_len = 0;
> -                int alt_ip_len = ASN1_STRING_length(name->d.iPAddress);
> +                GInetAddress * alt_ip = NULL;
> +                GInetAddress * ip = NULL;
> +                gchar * alt_ip_string = NULL;
> +                const guint8 * ip_binary = NULL;
> +                int alt_ip_len = 0;
> +                int ip_len = 0;
> +
>                  found_dns_name = 1;
>
> -                // only IpV4 supported
> -                if (inet_aton(hostname, &addr)) {
> -                    addr_len = sizeof(struct in_addr);
> -                }
> +                ip = g_inet_address_new_from_string(hostname);
> +                ip_len = g_inet_address_get_native_size(ip);
> +                ip_binary = g_inet_address_to_bytes(ip);
> +
> +                alt_ip_len = ASN1_STRING_length(name->d.iPAddress);
> +
> +                if ((ip_len == alt_ip_len) &&
> +                   (memcmp(ASN1_STRING_data(name->d.iPAddress), ip_binary, ip_len)) == 0) {
> +                    alt_ip = g_inet_address_new_from_bytes(ASN1_STRING_data(name->d.iPAddress),
> +                                                           g_inet_address_get_family(ip));
> +                    alt_ip_string = g_inet_address_to_string(alt_ip);
> +                    printf("alt name IP match=%s\n", alt_ip_string);

Is there a specific reason for changing the g_debug() to printf()?

>
> -                if ((addr_len == alt_ip_len)&&
> -                    !memcmp(ASN1_STRING_data(name->d.iPAddress), &addr, addr_len)) {
> -                    spice_debug("alt name IP match=%s",
> -                                inet_ntoa(*((struct in_addr*)ASN1_STRING_data(name->d.dNSName))));
> +                    g_free(alt_ip_string);
>                      GENERAL_NAMES_free(subject_alt_names);
>                      return 1;
>                  }
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Seems good, ACK!

-- 
Fabiano Fidêncio