Opened 8 years ago

Closed 2 years ago

#8061 closed enhancement (fixed)

Let the user select trusted ciphers for TLS

Reported by: ben Owned by: MarkDoliner
Milestone: 2.10.11 Component: libpurple
Version: 2.5.3 Keywords: ssl, tls
Cc: belmyst

Description

At the moment, pidgin will encrypt TLS connections using algorithms that can be as weak as DES:

  • With NSS, these ciphers are explicitly added to the cipher preference list (see ll. 142--153 in libpurple/plugins/ssl/ssl-nss.c), including DES.
  • With GnuTLS, gnutls_cipher_set_priority() isn't called, which I think implicitly causes the use of GnuTLS's default cipher priorities. Those also include old, less trusted ciphers like 3DES if I interpret lib/gnutls_priority.c from the official GnuTLS distribution correctly).

This behavior enables passive attacks on the TLS encryption between client and server if the server only supports weak ciphers. The user should be able to decide which ciphers to trust, and pidgin should refuse a connection to a server which doesn't support any of those ciphers.

Attachments (2)

patch-gnutls-ciphersuites-and-tlsv2.diff (1.3 KB) - added by belmyst 3 years ago.
Enabling Perfect Forward Secrecy ciphers in GnuTLS (plus additional tuning)
patch-nss-ciphersuites-and-tlsv2.diff (4.0 KB) - added by belmyst 3 years ago.
Enabling Perfect Forward Secrecy ciphers (including patch for #15909)

Download all attachments as: .zip

Change History (19)

comment:1 Changed 3 years ago by elrond

General

With the current developments, it would be really good to have this. I don't want to offer MD5 or RC4 to any server these days.

Cipher Suites

That said, it probably would be good to also be able to change the order of ciphers. The order I have seen on debian backport's pidgin is like:

  1. TLS_RSA_WITH_AES_256_CBC_SHA
  2. TLS_DHE_RSA_WITH_AES_256_CBC_SHA
  3. TLS_DHE_RSA_WITH_AES_128_CBC_SHA

With good DH params, one would really prefer 2 instead of 1 for forward secrecy (yes, OTR makes things better, but that's not the topic here).

Protocol Versions

NSS 3.14 (shipped in debian) supports TLS 1.1, while the handshake happening on the wire only indicates 1.0. Pleaser offer the maximum version supported by NSS to the server. TLS 1.1 fixes some issues and 1.2 brings the GCM ciphers. See #15909

Last edited 3 years ago by elrond (previous) (diff)

comment:2 Changed 3 years ago by elrond

Sorting ciphers

For GnuTLS it is easy to sort ciphers. The xmpp manifesto suggests to prefer ciphers with forward secrecy, so let's do that.

Old priority string: NORMAL:%SSL3_RECORD_VERSION New priority string: +PFS:+NORMAL:%SSL3_RECORD_VERSION

This only changes the order and should not affect anything else. Please consider merging this change soon.

comment:3 Changed 3 years ago by belmyst

According to http://blog.lighttpd.net/gnutls-priority-strings.html, +PFS:+NORMAL:%SSL3_RECORD_VERSION includes several out of date or insecure ciphers:

TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA	ECDHE-ECDSA	3DES-CBC	SHA1
TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA	ECDHE-RSA	3DES-CBC	SHA1
TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA	DHE-RSA	3DES-CBC	SHA1
TLS_DHE_DSS_WITH_AES_128_GCM_SHA256	DHE-DSS	AES-128-GCM	AEAD
TLS_DHE_DSS_WITH_AES_256_GCM_SHA384	DHE-DSS	AES-256-GCM	AEAD
TLS_DHE_DSS_WITH_CAMELLIA_128_GCM_SHA256	DHE-DSS	CAMELLIA-128-GCM	AEAD
TLS_DHE_DSS_WITH_CAMELLIA_256_GCM_SHA384	DHE-DSS	CAMELLIA-256-GCM	AEAD
TLS_DHE_DSS_WITH_AES_128_CBC_SHA	DHE-DSS	AES-128-CBC	SHA1
TLS_DHE_DSS_WITH_AES_128_CBC_SHA256	DHE-DSS	AES-128-CBC	SHA256
TLS_DHE_DSS_WITH_AES_256_CBC_SHA	DHE-DSS	AES-256-CBC	SHA1
TLS_DHE_DSS_WITH_AES_256_CBC_SHA256	DHE-DSS	AES-256-CBC	SHA256
TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA	DHE-DSS	CAMELLIA-128-CBC	SHA1
TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA256	DHE-DSS	CAMELLIA-128-CBC	SHA256
TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA	DHE-DSS	CAMELLIA-256-CBC	SHA1
TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA256	DHE-DSS	CAMELLIA-256-CBC	SHA256
TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA	DHE-DSS	3DES-CBC	SHA1
TLS_DHE_DSS_WITH_RC4_128_SHA	DHE-DSS	ARCFOUR-128	SHA1
TLS_RSA_WITH_3DES_EDE_CBC_SHA	RSA	3DES-CBC	SHA1
TLS_RSA_WITH_RC4_128_MD5	RSA	ARCFOUR-128	MD5

To decide, I used

as guides.

A previous version of my patch proposed +PFS:!3DES-CBC:!DHE-DSS:!CURVE-SECP192R1:!CURVE-SECP224R1:!MD5:+RSA:%SSL3_RECORD_VERSION as a fix, but this implies that GnuTLS 3.2.4 or higher is needed to compile (which is a no-no on e.g. Ubuntu 12.04 LTS).

I propose instead to use SECURE128:-RSA:+RSA:!DHE-DSS:%SSL3_RECORD_VERSION:

  • It should not change version requirements, unlike PFS.
  • It already kills 3-DES, RC4 and MD5 (as required by comment #2). Additionally, I killed DHE-DSS suites (does anyone use them?).
  • The -RSA:+RSA forces non-PFS suites to the bottom.

Regarding NSS, I've made a patch including

  • where possible, the same ciphersuites (based onto GnuTLS's offered key exchanges, ciphers and hash algorithms).
  • the changes proposed for #15909, enabling TLSv1.1 and higher.

It's my first patch here, so all and every comment is more than welcome :) I would particularly appreciate feedback on whether additional ciphers should (not) be included (especially RC4).

Last edited 3 years ago by belmyst (previous) (diff)

comment:4 Changed 3 years ago by Robby

  • Milestone set to Patches Needing Review

Changed 3 years ago by belmyst

Enabling Perfect Forward Secrecy ciphers in GnuTLS (plus additional tuning)

Changed 3 years ago by belmyst

Enabling Perfect Forward Secrecy ciphers (including patch for #15909)

comment:5 Changed 3 years ago by belmyst

Yahoo: "Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)" MSN: "Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xc027)" Sametime: see http://www-10.lotus.com/ldd/stwiki.nsf/page.xsp?documentId=74F93DA03010FE7185257C39006511E3&action=openDocument Testing these patches on other services would be much appreciated.

Last edited 3 years ago by belmyst (previous) (diff)

comment:6 Changed 3 years ago by juergenhecht

Hi guys, at the moment I use these as my default preferred ciphers in file: /libpurple/plugins/ssl/ssl-nss.c

// PFS w/ GCM
SSL_CipherPrefSetDefault(TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_DHE_RSA_WITH_AES_128_GCM_SHA256, 1);

// PFS w/ CBC
SSL_CipherPrefSetDefault(TLS_DHE_RSA_WITH_AES_256_CBC_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_DHE_RSA_WITH_AES_128_CBC_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, 1);
SSL_CipherPrefSetDefault(TLS_DHE_RSA_WITH_AES_256_CBC_SHA, 1);
SSL_CipherPrefSetDefault(TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, 1);
SSL_CipherPrefSetDefault(TLS_DHE_RSA_WITH_AES_128_CBC_SHA, 1);

// no PFS
SSL_CipherPrefSetDefault(TLS_RSA_WITH_AES_256_CBC_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_RSA_WITH_AES_128_GCM_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_RSA_WITH_AES_128_CBC_SHA256, 1);
SSL_CipherPrefSetDefault(TLS_RSA_WITH_AES_256_CBC_SHA, 1);
SSL_CipherPrefSetDefault(TLS_RSA_WITH_AES_128_CBC_SHA, 1);

comment:7 Changed 3 years ago by MarkDoliner

  • Owner set to MarkDoliner

I'm curious why you chose SECURE128 for gnutls? It seems like that excludes 256 bit ciphers. Are there problems with 256 bit ciphers? It seems like SECURE256 or NORMAL would be a better choice.

Also, does anyone know why we include %SSL3_RECORD_VERSION at the end of the cipher list? According to http://gnutls.org/manual/html_node/Priority-Strings.html it is the default.

comment:8 follow-up: Changed 3 years ago by MarkDoliner

Still looking at just the patch for gnutls so far. The patch changes the priority string from this:

NORMAL:%SSL3_RECORD_VERSION

To this:

SECURE128:-RSA:+RSA:!DHE-DSS:%SSL3_RECORD_VERSION

There are three changes:

  1. Switching the initial set of ciphers from NORMAL to SECURE128.
  2. Deprioritizing RSA ciphers.
  3. Removing all DHE-DSS ciphers.

Regarding the first change, in my last comment I said:

I'm curious why you chose SECURE128 for gnutls? It seems like that excludes 256 bit ciphers.

From my testing that appears to be true in GnuTLS 2.12.23 (possibly a bug?), but it's not true in GnuTLS 3.2.11. The behavior in 3.2.11 matches the documentation, which says that SECURE128 includes 128 and higher.

And in GnuTLS 3.2.11, using SECURE128 instead of NORMAL does disable a few ciphers:

  • TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA
  • TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
  • TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_ECDHE_RSA_WITH_RC4_128_SHA
  • TLS_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_RSA_WITH_RC4_128_SHA
  • TLS_RSA_WITH_RC4_128_MD5
  • TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA
  • TLS_DHE_DSS_WITH_RC4_128_SHA

So that's cool. It also tweaks the order slightly (nothing too important--just sorts some 128 bit ciphers ahead of 258 bit ciphers). So switching from NORMAL to SECURE128 seems fine to me.

Regarding the second and third change (deprioritizing RSA and removing all DHE-DSS), what's the reason for these?

I don't see a strong need to change the gnutls ciphers in our 2.x.y branch. The only cipher which https://www.howsmyssl.com/ complains about is TLS_DHE_DSS_WITH_RC4_128_SHA, and I think this is actually a bug in https://www.howsmyssl.com/ (see https://savannah.gnu.org/support/?108577 and https://github.com/jmhodges/howsmyssl/issues/35 and https://github.com/jmhodges/howsmyssl/commit/fe3db64430d4792e0d3be0f60a385dbb6cd6cf1b). Using SECURE128 in master (which will eventually be released as 3.0.0) isn't a bad idea. Though I'm a bit wary in general for us to be specifying which ciphers we want. I'd prefer for this to happen at a deeper level (either gnutls decides for us, or the OS decides). That seems like a better way for Pidgin to seamlessly adapt to changing ciphers without us needing to be encryption experts.

Another note about the patch: It changes the final fallback priority string to match the default priority string. It doesn't make sense to try to set the same string twice--if it failed the first time it's going to fail the second time. Anyway, that's minor and not really important.

comment:9 in reply to: ↑ 8 Changed 3 years ago by belmyst

Replying to MarkDoliner:

There are three changes:

  1. Switching the initial set of ciphers from NORMAL to SECURE128.
  2. Deprioritizing RSA ciphers.
  3. Removing all DHE-DSS ciphers.

Regarding the first change, in my last comment I said:

I'm curious why you chose SECURE128 for gnutls? It seems like that excludes 256 bit ciphers.

From my testing that appears to be true in GnuTLS 2.12.23 (possibly a bug?), but it's not true in GnuTLS 3.2.11. The behavior in 3.2.11 matches the documentation, which says that SECURE128 includes 128 and higher.

To workaround this, we could change the priority string to SECURE256:SECURE128:-RSA:+RSA:!DHE-DSS:%SSL3_RECORD_VERSION.

And in GnuTLS 3.2.11, using SECURE128 instead of NORMAL does disable a few ciphers:

  • TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA
  • TLS_ECDHE_ECDSA_WITH_RC4_128_SHA
  • TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_ECDHE_RSA_WITH_RC4_128_SHA
  • TLS_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_RSA_WITH_RC4_128_SHA
  • TLS_RSA_WITH_RC4_128_MD5
  • TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA
  • TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA
  • TLS_DHE_DSS_WITH_RC4_128_SHA

So that's cool. It also tweaks the order slightly (nothing too important--just sorts some 128 bit ciphers ahead of 258 bit ciphers). So switching from NORMAL to SECURE128 seems fine to me.

That's exactly what I intended, to disable any ciphers using MD5, 3DES or RC4.

Regarding the second and third change (deprioritizing RSA and removing all DHE-DSS), what's the reason for these?

DHE-DSS is just because I am not aware of any service using it. As it was my opinion to disable it, I'll be glad to remove that change if it's needed. Regarding RSA, I wanted to make sure that forward secrecy ciphersuites are prioritized. If you check Lighttpd's list, the ordering is as follows: ECDHE-ECDSA, ECDHE-RSA, RSA, DHE-RSA and DHE-DSS.

I don't see a strong need to change the gnutls ciphers in our 2.x.y branch. The only cipher which https://www.howsmyssl.com/ complains about is TLS_DHE_DSS_WITH_RC4_128_SHA, and I think this is actually a bug in https://www.howsmyssl.com/ (see https://savannah.gnu.org/support/?108577 and https://github.com/jmhodges/howsmyssl/issues/35 and https://github.com/jmhodges/howsmyssl/commit/fe3db64430d4792e0d3be0f60a385dbb6cd6cf1b). Using SECURE128 in master (which will eventually be released as 3.0.0) isn't a bad idea. Though I'm a bit wary in general for us to be specifying which ciphers we want. I'd prefer for this to happen at a deeper level (either gnutls decides for us, or the OS decides). That seems like a better way for Pidgin to seamlessly adapt to changing ciphers without us needing to be encryption experts.

Another note about the patch: It changes the final fallback priority string to match the default priority string. It doesn't make sense to try to set the same string twice--if it failed the first time it's going to fail the second time. Anyway, that's minor and not really important.

comment:10 Changed 3 years ago by MarkDoliner

belmyst: Cool, thanks for your response. I'm still working on the GnuTLS side of this. Dealing with the behavior differences between old GnuTLS and new GnuTLS is a pain. FYI this is a busy week for me so I might not check anything in until... later. But hopefully before the end of next week.

comment:11 Changed 3 years ago by Mark Doliner <mark@…>

(In [76a2a6d75768]):
Specify a different set of encryption ciphers for TLS connections when using GnuTLS.

Thanks to elrond and belmyst on Trac.

Refs #8061

comment:12 Changed 2 years ago by datallah

I wrote a plugin (https://pidgin.im/~datallah/nss-prefs.c) that allows customization of the NSS cipher suites.

Without 115560993ff5 the UI is pretty horrible, but it still works (and you can tweak the list in prefs.xml manually).

It'll most likely make it in the next release.

comment:13 Changed 2 years ago by Daniel Atallah <datallah@…>

(In [1cdc641d433e]):
Add "NSS Preferences" plugin which allows configuration Min/Max? TLS version and Ciphers.

  • The TLS version settings require NSS 3.14 or newer

Refs #8061

comment:14 Changed 2 years ago by datallah

Given that there's now a way to specify the ciphers for GnuTLS (via the PURPLE_GNUTLS_PRIORITIES environment variable) and for NSS (using the new plugin), I think we can consider this fixed for 2.10.11.

comment:15 Changed 2 years ago by Robby

  • Milestone changed from Patches Needing Review to 2.10.11

Shouldn’t this ticket be closed as "fixed" and 2.10.11 be applied as the milestone?

comment:16 Changed 2 years ago by MarkDoliner

  • Milestone changed from 2.10.11 to 2.10.12

comment:17 Changed 2 years ago by datallah

  • Milestone changed from 2.10.12 to 2.10.11
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!