[Spice-devel] [spice-gtk 06/13] http-proxy: add https proxy

[Christophe Fergeau]

On Mon, Feb 03, 2014 at 07:02:37PM +0100, Marc-André Lureau wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> This will require glib 2.28 for GTls support, atm
> ---
>  gtk/spice-session.c    |   3 +
>  gtk/wocky-http-proxy.c | 166 +++++++++++++++++++++++++++++++++++++++++--------
>  gtk/wocky-http-proxy.h |  14 +++++
>  3 files changed, 157 insertions(+), 26 deletions(-)
> 
> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
> index ae14a1f..6ac397c 100644
> --- a/gtk/spice-session.c
> +++ b/gtk/spice-session.c
> @@ -636,6 +636,9 @@ static void spice_session_class_init(SpiceSessionClass *klass)
>  #if GLIB_CHECK_VERSION(2, 26, 0)
>      _wocky_http_proxy_get_type();
>  #endif
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +    _wocky_https_proxy_get_type();
> +#endif
>  
>      gobject_class->dispose      = spice_session_dispose;
>      gobject_class->finalize     = spice_session_finalize;
> diff --git a/gtk/wocky-http-proxy.c b/gtk/wocky-http-proxy.c
> index 7210859..25f2af5 100644
> --- a/gtk/wocky-http-proxy.c
> +++ b/gtk/wocky-http-proxy.c
> @@ -1,7 +1,9 @@
>   /* wocky-http-proxy.c: Source for WockyHttpProxy
>   *
>   * Copyright (C) 2010 Collabora, Ltd.
> + * Copyright (C) 2014 Red Hat, Inc.
>   * @author Nicolas Dufresne <nicolas.dufresne at collabora.co.uk>
> + * @author Marc-André Lureau <marcandre.lureau at redhat.com>
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -40,13 +42,13 @@ static void wocky_http_proxy_iface_init (GProxyInterface *proxy_iface);
>  
>  #define wocky_http_proxy_get_type _wocky_http_proxy_get_type
>  G_DEFINE_TYPE_WITH_CODE (WockyHttpProxy, wocky_http_proxy, G_TYPE_OBJECT,
> -    G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
> -      wocky_http_proxy_iface_init)
> -    g_io_extension_point_set_required_type (
> -      g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
> -      G_TYPE_PROXY);
> -    g_io_extension_point_implement (G_PROXY_EXTENSION_POINT_NAME,
> -      g_define_type_id, "http", 0))
> +  G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
> +    wocky_http_proxy_iface_init)
> +  g_io_extension_point_set_required_type (
> +    g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
> +    G_TYPE_PROXY);
> +  g_io_extension_point_implement (G_PROXY_EXTENSION_POINT_NAME,
> +    g_define_type_id, "http", 0))
>  
>  static void
>  wocky_http_proxy_init (WockyHttpProxy *proxy)
> @@ -180,10 +182,34 @@ wocky_http_proxy_connect (GProxy *proxy,
>  {
>    GInputStream *in;
>    GOutputStream *out;
> -  GDataInputStream *data_in;
> -  gchar *buffer;
> +  GDataInputStream *data_in = NULL;
> +  gchar *buffer = NULL;
>    gboolean has_cred;
>  
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +  if (WOCKY_IS_HTTPS_PROXY (proxy))
> +    {
> +      GIOStream *tlsconn;
> +
> +      tlsconn = g_tls_client_connection_new (io_stream,
> +                                             G_SOCKET_CONNECTABLE(proxy_address),
> +                                             error);
> +      if (!tlsconn)
> +          goto error;
> +
> +      GTlsCertificateFlags tls_validation_flags = G_TLS_CERTIFICATE_VALIDATE_ALL;
> +#ifdef DEBUG
> +      tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA + G_TLS_CERTIFICATE_BAD_IDENTITY;
> +#endif
> +      g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn),
> +                                                    tls_validation_flags);
> +      if (!g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn), cancellable, error))
> +          goto error;
> +
> +      io_stream = tlsconn;

Missed that initially, but tlsconn is leaked in error cases, and I think we
will leak a reference to it in success case too. My understanding is that
we don't own the io_stream passed as an argument, so we need to ref it in
the http case, but in the https case, we already own a ref on the io_stream
(which is tlsconn), so we ref it one too many.

> +
> +static void
>  wocky_http_proxy_connect_async (GProxy *proxy,
>      GIOStream *io_stream,
>      GProxyAddress *proxy_address,
> @@ -300,34 +361,55 @@ wocky_http_proxy_connect_async (GProxy *proxy,
> +
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +  if (WOCKY_IS_HTTPS_PROXY (proxy))
> +    {
> +      GError *error = NULL;
> +      GIOStream *tlsconn;
> +
> +      tlsconn = g_tls_client_connection_new (io_stream,
> +                                             G_SOCKET_CONNECTABLE(proxy_address),
> +                                             &error);
> +      if (error)
> +        {
> +          complete_async_from_error (data, error);

tlsconn is leaked here (dunno if it's guaranteed to be NULL when error is
set).

> +          return;
> +        }
> +
> +      g_return_if_fail (tlsconn != NULL);
> +
> +      GTlsCertificateFlags tls_validation_flags = G_TLS_CERTIFICATE_VALIDATE_ALL;
> +#ifdef DEBUG
> +      tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA + G_TLS_CERTIFICATE_BAD_IDENTITY;
> +#endif
> +      g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (tlsconn),
> +                                                    tls_validation_flags);
> +      g_tls_connection_handshake_async (G_TLS_CONNECTION (tlsconn),
> +                                        G_PRIORITY_DEFAULT, cancellable,
> +                                        handshake_completed, data);

A g_object_unref(tlsconn) is probably needed here.


Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140212/f63c5021/attachment.pgp>