utterly insecure GNUTLS settings

[onlyjob]

As reported in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=803204:

Since I changed my XMPP server, Zabbix failed to send alerts via XMPP
with "tls handshake failed". The XMPP server said "no shared cipher".
After some research to see how Zabbix do its job I ended up into this
library. I confirmed there is no way to setup the ciphers into Zabbix,
but I was then astonished to see them hardcoded and very low grade in
libiksemel:

       const int protocol_priority[] = { GNUTLS_TLS1, GNUTLS_SSL3, 0 };
       const int kx_priority[] = { GNUTLS_KX_RSA, 0 };
       const int cipher_priority[] = { GNUTLS_CIPHER_3DES_CBC, 
GNUTLS_CIPHER_ARCFOUR, 0};
       const int comp_priority[] = { GNUTLS_COMP_ZLIB, GNUTLS_COMP_NULL, 
0 };
       const int mac_priority[] = { GNUTLS_MAC_SHA, GNUTLS_MAC_MD5, 0 };

SSL3, 3DES, RC4, SSL compression… With this setting not only low grade
ciphers are available, but higher grades are disabled. So this is a
major security issue, also affecting stable.

The following patch fixes the security problem (and compatibility
problem with servers rejecting low grade ciphers). You should
nevertheless proofread my choices, as I'm no security expert. The patch
does not change the original priority lists because I failed somehow to
fix them all, so I replaced it by a priority string (which is a
non-obsolete method to do it anyway).

Index: libiksemel-1.4/src/stream.c
===================================================================
--- libiksemel-1.4.orig/src/stream.c
+++ libiksemel-1.4/src/stream.c
@@ -63,11 +63,7 @@ tls_pull (iksparser *prs, char *buffer,
 static int
 handshake (struct stream_data *data)
 {
-   const int protocol_priority[] = { GNUTLS_TLS1, GNUTLS_SSL3, 0 };
-   const int kx_priority[] = { GNUTLS_KX_RSA, 0 };
-   const int cipher_priority[] = { GNUTLS_CIPHER_3DES_CBC, GNUTLS_CIPHER_ARCFOUR, 0};
-   const int comp_priority[] = { GNUTLS_COMP_ZLIB, GNUTLS_COMP_NULL, 0 };
-   const int mac_priority[] = { GNUTLS_MAC_SHA, GNUTLS_MAC_MD5, 0 };
+   const char *priority_string = "SECURE256:+SECURE192:-VERS-TLS-ALL:+VERS-TLS1.2";
    int ret;

    if (gnutls_global_init () != 0)
@@ -80,11 +76,7 @@ handshake (struct stream_data *data)
        gnutls_certificate_free_credentials (data->cred);
        return IKS_NOMEM;
    }
-   gnutls_protocol_set_priority (data->sess, protocol_priority);
-   gnutls_cipher_set_priority(data->sess, cipher_priority);
-   gnutls_compression_set_priority(data->sess, comp_priority);
-   gnutls_kx_set_priority(data->sess, kx_priority);
-   gnutls_mac_set_priority(data->sess, mac_priority);
+   gnutls_priority_set_direct(data->sess, priority_string, NULL);
    gnutls_credentials_set (data->sess, GNUTLS_CRD_CERTIFICATE, data->cred);

    gnutls_transport_set_push_function (data->sess, (gnutls_push_func) tls_push);

Regards.
Marc Dequènes

[chris001]

Paging @meduketto I think you should pull @onlyjob Marc Dequenes security fix into the main branch of this jabber code, mate..