From f0e10fe4100ec62e076d70727eadbe235014c8c6 Mon Sep 17 00:00:00 2001
From: Gu1 <aeroxteam@gmail.com>
Date: Tue, 15 Feb 2011 00:11:39 +0100
Subject: [PATCH] Fix a problem with certificate verification.

It was possible to completely bypass ssl_verify because the function
responsible for checking the certificate was only called when the
server requested a client certificate.
---
 src/core/wee-hook.c          |   34 ++++-
 src/core/wee-hook.h          |    8 +-
 src/core/wee-network.c       |    2 +
 src/plugins/irc/irc-server.c |  338 ++++++++++++++++++++++--------------------
 4 files changed, 216 insertions(+), 166 deletions(-)

diff --git a/src/core/wee-hook.c b/src/core/wee-hook.c
index 67478bd..7a2f580 100644
--- a/src/core/wee-hook.c
+++ b/src/core/wee-hook.c
@@ -1700,6 +1700,38 @@ hook_connect (struct t_weechat_plugin *plugin, const char *proxy, const char *ad
 }
 
 /*
+ * hook_connect_gnutls_verify_certificates: verify server certificate
+ */
+
+#ifdef HAVE_GNUTLS
+int
+hook_connect_gnutls_verify_certificates (gnutls_session_t tls_session)
+{
+    struct t_hook *ptr_hook;
+    int rc;
+    
+    rc = -1;
+    ptr_hook = weechat_hooks[HOOK_TYPE_CONNECT];
+    while (ptr_hook)
+    {
+        /* looking for the right hook using to the gnutls session pointer */
+        if (!ptr_hook->deleted
+            && HOOK_CONNECT(ptr_hook, gnutls_sess)
+            && (*(HOOK_CONNECT(ptr_hook, gnutls_sess)) == tls_session))
+        {
+            rc = (int) (HOOK_CONNECT(ptr_hook, gnutls_cb))
+                (ptr_hook->callback_data, tls_session, NULL, 0,
+                 NULL, 0, NULL, GNUTLS_CB_MODE_VERIFY);
+            break;
+        }
+        ptr_hook = ptr_hook->next_hook;
+    }
+    
+    return rc;
+}
+#endif
+
+/*
  * hook_connect_gnutls_set_certificates: set gnutls
  */
 
@@ -1725,7 +1757,7 @@ hook_connect_gnutls_set_certificates (gnutls_session_t tls_session,
         {
             rc = (int) (HOOK_CONNECT(ptr_hook, gnutls_cb))
                 (ptr_hook->callback_data, tls_session, req_ca, nreq,
-                 pk_algos, pk_algos_len, answer);
+                 pk_algos, pk_algos_len, answer, GNUTLS_CB_MODE_SETCERT);
             break;
         }
         ptr_hook = ptr_hook->next_hook;
diff --git a/src/core/wee-hook.h b/src/core/wee-hook.h
index 1c2e357..224f6af 100644
--- a/src/core/wee-hook.h
+++ b/src/core/wee-hook.h
@@ -202,10 +202,15 @@ typedef int (t_hook_callback_connect)(void *data, int status,
                                       const char *ip_address);
 
 #ifdef HAVE_GNUTLS
+enum t_gnutls_cb_mode {
+    GNUTLS_CB_MODE_SETCERT,
+    GNUTLS_CB_MODE_VERIFY
+};
 typedef int (gnutls_callback_t)(void *data, gnutls_session_t tls_session,
                                 const gnutls_datum_t *req_ca, int nreq,
                                 const gnutls_pk_algorithm_t *pk_algos,
-                                int pk_algos_len, gnutls_retr_st *answer);
+                                int pk_algos_len, gnutls_retr_st *answer,
+                                enum t_gnutls_cb_mode cbmode);
 #endif
 
 struct t_hook_connect
@@ -416,6 +421,7 @@ extern int hook_connect_gnutls_set_certificates (gnutls_session_t tls_session,
                                                  const gnutls_pk_algorithm_t *pk_algos,
                                                  int pk_algos_len,
                                                  gnutls_retr_st *answer);
+extern int hook_connect_gnutls_verify_certificates (gnutls_session_t tls_session);
 #endif
 extern struct t_hook *hook_print (struct t_weechat_plugin *plugin,
                                   struct t_gui_buffer *buffer,
diff --git a/src/core/wee-network.c b/src/core/wee-network.c
index f74a90b..08fa691 100644
--- a/src/core/wee-network.c
+++ b/src/core/wee-network.c
@@ -94,6 +94,8 @@ network_init ()
         }
         free (ca_path);
     }
+    gnutls_certificate_set_verify_function (gnutls_xcred,
+                                            &hook_connect_gnutls_verify_certificates);
     gnutls_certificate_client_set_retrieve_function (gnutls_xcred,
                                                      &hook_connect_gnutls_set_certificates);
     network_init_ok = 1;
diff --git a/src/plugins/irc/irc-server.c b/src/plugins/irc/irc-server.c
index acbc1ca..01d9129 100644
--- a/src/plugins/irc/irc-server.c
+++ b/src/plugins/irc/irc-server.c
@@ -2797,6 +2797,10 @@ irc_server_create_buffer (struct t_irc_server *server)
 }
 
 #ifdef HAVE_GNUTLS
+enum t_gnutls_cb_mode {
+    GNUTLS_CB_MODE_SETCERT,
+    GNUTLS_CB_MODE_VERIFY
+};
 /*
  * irc_server_gnutls_callback: gnutls callback called during handshake
  *
@@ -2805,7 +2809,8 @@ int
 irc_server_gnutls_callback (void *data, gnutls_session_t tls_session,
                             const gnutls_datum_t *req_ca, int nreq,
                             const gnutls_pk_algorithm_t *pk_algos,
-                            int pk_algos_len, gnutls_retr_st *answer)
+                            int pk_algos_len, gnutls_retr_st *answer,
+                            enum t_gnutls_cb_mode cbmode)
 {
     struct t_irc_server *server;
     gnutls_retr_st tls_struct;
@@ -2837,207 +2842,212 @@ irc_server_gnutls_callback (void *data, gnutls_session_t tls_session,
     hostname = server->current_address;
     hostname_match = 0;
     
-    weechat_printf (server->buffer,
-                    _("gnutls: connected using %d-bit Diffie-Hellman shared "
-                      "secret exchange"),
-                    IRC_SERVER_OPTION_INTEGER (server,
-                                               IRC_SERVER_OPTION_SSL_DHKEY_SIZE));
-    if (gnutls_certificate_verify_peers2 (tls_session, &status) < 0)
+    if (cbmode == GNUTLS_CB_MODE_VERIFY) 
     {
         weechat_printf (server->buffer,
-                        _("%sgnutls: error while checking peer's certificate"),
-                        weechat_prefix ("error"));
-        rc = -1;
-    }
-    else
-    {
-        /* some checks */
-        if (status & GNUTLS_CERT_INVALID)
+                        _("gnutls: connected using %d-bit Diffie-Hellman shared "
+                          "secret exchange"),
+                        IRC_SERVER_OPTION_INTEGER (server,
+                                                   IRC_SERVER_OPTION_SSL_DHKEY_SIZE));
+        if (gnutls_certificate_verify_peers2 (tls_session, &status) < 0)
         {
             weechat_printf (server->buffer,
-                            _("%sgnutls: peer's certificate is NOT trusted"),
+                            _("%sgnutls: error while checking peer's certificate"),
                             weechat_prefix ("error"));
             rc = -1;
         }
         else
         {
-            weechat_printf (server->buffer,
-                            _("gnutls: peer's certificate is trusted"));
-        }
-        if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
-        {
-            weechat_printf (server->buffer,
-                            _("%sgnutls: peer's certificate issuer is unknown"),
-                            weechat_prefix ("error"));
-            rc = -1;
-        }
-        if (status & GNUTLS_CERT_REVOKED)
-        {
-            weechat_printf (server->buffer,
-                            _("%sgnutls: the certificate has been revoked"),
-                            weechat_prefix ("error"));
-            rc = -1;
-        }
-        
-        /* check certificates */
-        if (gnutls_x509_crt_init (&cert_temp) >= 0)
-        {
-            cert_list = gnutls_certificate_get_peers (tls_session, &cert_list_len);
-            if (cert_list)
+            /* some checks */
+            if (status & GNUTLS_CERT_INVALID)
+            {
+                weechat_printf (server->buffer,
+                                _("%sgnutls: peer's certificate is NOT trusted"),
+                                weechat_prefix ("error"));
+                rc = -1;
+            }
+            else
+            {
+                weechat_printf (server->buffer,
+                                _("gnutls: peer's certificate is trusted"));
+            }
+            if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
+            {
+                weechat_printf (server->buffer,
+                                _("%sgnutls: peer's certificate issuer is unknown"),
+                                weechat_prefix ("error"));
+                rc = -1;
+            }
+            if (status & GNUTLS_CERT_REVOKED)
             {
                 weechat_printf (server->buffer,
-                                NG_("gnutls: receiving %d certificate",
-                                    "gnutls: receiving %d certificates",
-                                    cert_list_len),
-                                cert_list_len);
-                for (i = 0, j = (int) cert_list_len; i < j; i++)
+                                _("%sgnutls: the certificate has been revoked"),
+                                weechat_prefix ("error"));
+                rc = -1;
+            }
+            
+            /* check certificates */
+            if (gnutls_x509_crt_init (&cert_temp) >= 0)
+            {
+                cert_list = gnutls_certificate_get_peers (tls_session, &cert_list_len);
+                if (cert_list)
                 {
-                    if (gnutls_x509_crt_import (cert_temp, &cert_list[i], GNUTLS_X509_FMT_DER) >= 0)
+                    weechat_printf (server->buffer,
+                                    NG_("gnutls: receiving %d certificate",
+                                        "gnutls: receiving %d certificates",
+                                        cert_list_len),
+                                    cert_list_len);
+                    for (i = 0, j = (int) cert_list_len; i < j; i++)
                     {
-                        /* checking if hostname matches in the first certificate */
-                        if (i == 0 && gnutls_x509_crt_check_hostname (cert_temp, hostname) != 0)
+                        if (gnutls_x509_crt_import (cert_temp, &cert_list[i], GNUTLS_X509_FMT_DER) >= 0)
                         {
-                            hostname_match = 1;
-                        }
+                            /* checking if hostname matches in the first certificate */
+                            if (i == 0 && gnutls_x509_crt_check_hostname (cert_temp, hostname) != 0)
+                            {
+                                hostname_match = 1;
+                            }
 #if LIBGNUTLS_VERSION_NUMBER >= 0x010706
-                        /* displaying infos about certificate */
+                            /* displaying infos about certificate */
 #if LIBGNUTLS_VERSION_NUMBER < 0x020400
-                        rinfo = gnutls_x509_crt_print (cert_temp, GNUTLS_X509_CRT_ONELINE, &cinfo);
+                            rinfo = gnutls_x509_crt_print (cert_temp, GNUTLS_X509_CRT_ONELINE, &cinfo);
 #else
-                        rinfo = gnutls_x509_crt_print (cert_temp, GNUTLS_CRT_PRINT_ONELINE, &cinfo);
+                            rinfo = gnutls_x509_crt_print (cert_temp, GNUTLS_CRT_PRINT_ONELINE, &cinfo);
 #endif
-                        if (rinfo == 0)
-                        {
-                            weechat_printf (server->buffer,
-                                            _(" - certificate[%d] info:"), i + 1);
-                            weechat_printf (server->buffer,
-                                            "   - %s", cinfo.data);
-                            gnutls_free (cinfo.data);
-                        }
+                            if (rinfo == 0)
+                            {
+                                weechat_printf (server->buffer,
+                                                _(" - certificate[%d] info:"), i + 1);
+                                weechat_printf (server->buffer,
+                                                "   - %s", cinfo.data);
+                                gnutls_free (cinfo.data);
+                            }
 #endif
-                        /* check expiration date */
-                        cert_time = gnutls_x509_crt_get_expiration_time (cert_temp);
-                        if (cert_time < time(NULL))
-                        {
-                            weechat_printf (server->buffer,
-                                            _("%sgnutls: certificate has expired"),
-                                            weechat_prefix ("error"));
-                            rc = -1;
-                        }
-                        /* check expiration date */
-                        cert_time = gnutls_x509_crt_get_activation_time (cert_temp);
-                        if (cert_time > time(NULL))
-                        {
-                            weechat_printf (server->buffer,
-                                            _("%sgnutls: certificate is not yet activated"),
-                                            weechat_prefix ("error"));
-                            rc = -1;
+                            /* check expiration date */
+                            cert_time = gnutls_x509_crt_get_expiration_time (cert_temp);
+                            if (cert_time < time(NULL))
+                            {
+                                weechat_printf (server->buffer,
+                                                _("%sgnutls: certificate has expired"),
+                                                weechat_prefix ("error"));
+                                rc = -1;
+                            }
+                            /* check expiration date */
+                            cert_time = gnutls_x509_crt_get_activation_time (cert_temp);
+                            if (cert_time > time(NULL))
+                            {
+                                weechat_printf (server->buffer,
+                                                _("%sgnutls: certificate is not yet activated"),
+                                                weechat_prefix ("error"));
+                                rc = -1;
+                            }
                         }
                     }
-                }
-                if (hostname_match == 0)
-                {
-                    weechat_printf (server->buffer,
-                                    _("%sgnutls: the hostname in the "
-                                      "certificate does NOT match \"%s\""),
-                                    weechat_prefix ("error"), hostname);
-                    rc = -1;
+                    if (hostname_match == 0)
+                    {
+                        weechat_printf (server->buffer,
+                                        _("%sgnutls: the hostname in the "
+                                          "certificate does NOT match \"%s\""),
+                                        weechat_prefix ("error"), hostname);
+                        rc = -1;
+                    }
                 }
             }
         }
     }
-    
-    /* using client certificate if it exists */
-    cert_path0 = (char *) IRC_SERVER_OPTION_STRING(server,
-                                                   IRC_SERVER_OPTION_SSL_CERT);
-    if (cert_path0 && cert_path0[0])
+    else
     {
-        weechat_dir = weechat_info_get ("weechat_dir", "");
-        cert_path1 = weechat_string_replace (cert_path0, "%h", weechat_dir);
-        cert_path2 = (cert_path1) ?
-            weechat_string_expand_home (cert_path1) : NULL;
-        
-        if (cert_path2)
+        /* using client certificate if it exists */
+        cert_path0 = (char *) IRC_SERVER_OPTION_STRING(server,
+                                                       IRC_SERVER_OPTION_SSL_CERT);
+        if (cert_path0 && cert_path0[0])
         {
-            cert_str = weechat_file_get_content (cert_path2);
-            if (cert_str)
+            weechat_dir = weechat_info_get ("weechat_dir", "");
+            cert_path1 = weechat_string_replace (cert_path0, "%h", weechat_dir);
+            cert_path2 = (cert_path1) ?
+                weechat_string_expand_home (cert_path1) : NULL;
+            
+            if (cert_path2)
             {
-                weechat_printf (server->buffer,
-                                _("gnutls: sending one certificate"));
-                
-                filedatum.data = (unsigned char *) cert_str;
-                filedatum.size = strlen (cert_str);
-                
-                /* certificate */
-                gnutls_x509_crt_init (&server->tls_cert);
-                gnutls_x509_crt_import (server->tls_cert, &filedatum,
-                                        GNUTLS_X509_FMT_PEM);
-                
-                /* key */
-                gnutls_x509_privkey_init (&server->tls_cert_key);
-                ret = gnutls_x509_privkey_import (server->tls_cert_key,
-                                                  &filedatum,
-                                                  GNUTLS_X509_FMT_PEM);
-                if (ret < 0)
-                {
-                    ret = gnutls_x509_privkey_import_pkcs8 (server->tls_cert_key,
-                                                            &filedatum,
-                                                            GNUTLS_X509_FMT_PEM,
-                                                            NULL,
-                                                            GNUTLS_PKCS_PLAIN);
-                }
-                if (ret < 0)
+                cert_str = weechat_file_get_content (cert_path2);
+                if (cert_str)
                 {
                     weechat_printf (server->buffer,
-                                    _("%sgnutls: invalid certificate \"%s\", "
-                                      "error: %s"),
-                                    weechat_prefix ("error"), cert_path2,
-                                    gnutls_strerror (ret));
-                    rc = -1;
-                }
-                else
-                {
-                    tls_struct.type = GNUTLS_CRT_X509;
-                    tls_struct.ncerts = 1;
-                    tls_struct.deinit_all = 0;
-                    tls_struct.cert.x509 = &server->tls_cert;
-                    tls_struct.key.x509 = server->tls_cert_key;
+                                    _("gnutls: sending one certificate"));
+                    
+                    filedatum.data = (unsigned char *) cert_str;
+                    filedatum.size = strlen (cert_str);
+                    
+                    /* certificate */
+                    gnutls_x509_crt_init (&server->tls_cert);
+                    gnutls_x509_crt_import (server->tls_cert, &filedatum,
+                                            GNUTLS_X509_FMT_PEM);
+                    
+                    /* key */
+                    gnutls_x509_privkey_init (&server->tls_cert_key);
+                    ret = gnutls_x509_privkey_import (server->tls_cert_key,
+                                                      &filedatum,
+                                                      GNUTLS_X509_FMT_PEM);
+                    if (ret < 0)
+                    {
+                        ret = gnutls_x509_privkey_import_pkcs8 (server->tls_cert_key,
+                                                                &filedatum,
+                                                                GNUTLS_X509_FMT_PEM,
+                                                                NULL,
+                                                                GNUTLS_PKCS_PLAIN);
+                    }
+                    if (ret < 0)
+                    {
+                        weechat_printf (server->buffer,
+                                        _("%sgnutls: invalid certificate \"%s\", "
+                                          "error: %s"),
+                                        weechat_prefix ("error"), cert_path2,
+                                        gnutls_strerror (ret));
+                        rc = -1;
+                    }
+                    else
+                    {
+                        tls_struct.type = GNUTLS_CRT_X509;
+                        tls_struct.ncerts = 1;
+                        tls_struct.deinit_all = 0;
+                        tls_struct.cert.x509 = &server->tls_cert;
+                        tls_struct.key.x509 = server->tls_cert_key;
 #if LIBGNUTLS_VERSION_NUMBER >= 0x010706
-                    /* client certificate info */
+                        /* client certificate info */
 #if LIBGNUTLS_VERSION_NUMBER < 0x020400
-                    rinfo = gnutls_x509_crt_print (server->tls_cert,
-                                                   GNUTLS_X509_CRT_ONELINE,
-                                                   &cinfo);
+                        rinfo = gnutls_x509_crt_print (server->tls_cert,
+                                                       GNUTLS_X509_CRT_ONELINE,
+                                                       &cinfo);
 #else
-                    rinfo = gnutls_x509_crt_print (server->tls_cert,
-                                                   GNUTLS_CRT_PRINT_ONELINE,
-                                                   &cinfo);
+                        rinfo = gnutls_x509_crt_print (server->tls_cert,
+                                                       GNUTLS_CRT_PRINT_ONELINE,
+                                                       &cinfo);
 #endif
-                    if (rinfo == 0)
-                    {
-                        weechat_printf (server->buffer,
-                                        _(" - client certificate info (%s):"),
-                                        cert_path2);
-                        weechat_printf (server->buffer, "  - %s", cinfo.data);
-                        gnutls_free (cinfo.data);
-                    }
+                        if (rinfo == 0)
+                        {
+                            weechat_printf (server->buffer,
+                                            _(" - client certificate info (%s):"),
+                                            cert_path2);
+                            weechat_printf (server->buffer, "  - %s", cinfo.data);
+                            gnutls_free (cinfo.data);
+                        }
 #endif
-                    memcpy (answer, &tls_struct, sizeof (gnutls_retr_st));
-                    free (cert_str);
+                        memcpy (answer, &tls_struct, sizeof (gnutls_retr_st));
+                        free (cert_str);
+                    }
+                }
+                else
+                {
+                    weechat_printf (server->buffer,
+                                    _("%sgnutls: unable to read certifcate \"%s\""),
+                                    weechat_prefix ("error"), cert_path2);
                 }
             }
-            else
-            {
-                weechat_printf (server->buffer,
-                                _("%sgnutls: unable to read certifcate \"%s\""),
-                                weechat_prefix ("error"), cert_path2);
-            }
+            
+            if (cert_path1)
+                free (cert_path1);
+            if (cert_path2)
+                free (cert_path2);
         }
-        
-        if (cert_path1)
-            free (cert_path1);
-        if (cert_path2)
-            free (cert_path2);
     }
     
     /* an error should stop the handshake unless the user doesn't care */
-- 
1.7.4

