GNOME Bugzilla – Bug 677717
Make 256-bit AES encryption a higher priority in the GnuTLS backend
Last modified: 2014-01-02 14:19:27 UTC
Created attachment 215975 [details] [review] Change the GnuTLS priority string from NORMAL to SECURE256 The GnuTLS priority string is used to set the priorities of the cryptographic ciphers that are used when negotiating a secure connection. Currently, glib-networking is using the NORMAL priority, which enables all "secure" ciphers, however only includes 256-bit AES as a fallback. The SECURE256 priority includes all the same ciphers as NORMAL, but the priority order is changed so the 256-bit AES cipher has a higher priority than the others. Currently, web browsers that depend on glib-networking through libsoup (and the GnuTLS backend) are defaulting to the weaker 128-bit AES cipher. Other browsers that I have tested (Firefox, links, and lynx) all default to the stronger 256-bit cipher. Attached is a patch to update the priority string to SECURE256 to make these clients use the stronger cipher.
I really want this too!
Comment on attachment 215975 [details] [review] Change the GnuTLS priority string from NORMAL to SECURE256 This makes sense, but from googling, it looks like SECURE256 breaks TLS 1.2 negotiation in gnutls 2.12.x: http://lists.gnu.org/archive/html/help-gnutls/2012-01/msg00006.html. ("breaks" meaning, it will negotiate TLS 1.2 but then not be able to speak it.) At least Fedora is still using gnutls 2.12.x (https://bugzilla.redhat.com/show_bug.cgi?id=726886), so this patch needs to check if we're building against 2.12 or 3.0, and only use SECURE256 in the 3.0 case. (Probably extracting "NORMAL:%COMPAT" / "SECURE256:%COMPAT" into a #define would simplify this.)
I'm a bit confused about that email thread that you linked to. First of all, it's never mentioned that using NORMAL instead of SECURE256 would fix this, or that SECURE256 was being used in the first place. Also, I went through the GnuTLS release logs and I believe they have fixed this bug in version 2.12.18. http://article.gmane.org/gmane.comp.encryption.gpg.gnutls.devel/5931
(In reply to comment #3) > I'm a bit confused about that email thread that you linked to. sorry, http://lists.gnu.org/archive/html/help-gnutls/2011-05/msg00025.html explains in more detail. I linked to the other post because that one explicitly mentioned that the problem was fixed in 3.0, and didn't notice that it didn't really explain the bug that much. > Also, I went through the > GnuTLS release logs and I believe they have fixed this bug in version 2.12.18. > > http://article.gmane.org/gmane.comp.encryption.gpg.gnutls.devel/5931 I don't think that's related. 2.12.18 still has the return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR); /* too bad we only support SHA1 and SHA256 */ line. But I can't find a server to test this against...
This site is what I've been using to show the cipher being used: https://www.fortify.net/sslcheck.html
That doesn't support TLS 1.2 though. (That's the hard part; there just aren't that many servers out there that support TLS 1.2...) Hm... maybe if I ran the gnutls 3.0 gnutls-serv against the gnutls 2.12 gnutls-cli...
eh, we'll just see if anyone complains that we've broken their site. :) fixed in master. thanks for the patch.
(In reply to comment #7) > eh, we'll just see if anyone complains that we've broken their site. :) > Hey, that would be me ;) This breaks at least bugzilla (SSL handshake failure) and Twitter (TLS socket failure something something), so I don't think it's right as it is.
Hm... I'm typing this into bugzilla from epiphany with glib-networking master right now... You sure it was this patch and not some other that broke it? (Twitter... sometimes mostly loads, just like always.)
oh, what gnutls?
(In reply to comment #10) > oh, what gnutls? I'm using bcdf79aea10f3f30817ce0b3e9b29dab435219c6 from git. I guess I should update?
(In reply to comment #11) > (In reply to comment #10) > > oh, what gnutls? > > I'm using bcdf79aea10f3f30817ce0b3e9b29dab435219c6 from git. I guess I should > update? In case you don't track git, this seems to be 3.0.18.
I am a huge fan of this commit. The only comment I have is that it might be a good idea to make those strings knobs instead of hard coding them. That way everyone can play.
It seems that the meaning of SECURE256 changed between 2.12 and 3.0; in 2.12 it means "prefer 256-bit ciphers", but in 3.0 it means "only accept 256-bit ciphers". It's not clear to me if there's an easy way to get the 2.12-era behavior in 3.0... I've reverted the patch for now.
(In reply to comment #13) > I am a huge fan of this commit. The only comment I have is that it might be a > good idea to make those strings knobs instead of hard coding them. That way > everyone can play. I'd be fine with some sort of API that let you enable/disable specific ciphersuites, TLS versions, etc. But it shouldn't use gnutls priority strings directly, since there will eventually be an NSS-based gtls backend too.
Created attachment 216413 [details] [review] Only use SECURE256 if the GnuTLS version is less than 3. This patch only initializes the gnutls session with the SECURE256 priority strings if the version is less than 3.0.0. I haven't been able to test thoroughly because I don't have version 3 installed. Feedback welcome.
Created attachment 216415 [details] [review] Only use SECURE256 if the GnuTLS version is less than 3 botched the last patch, this one should be fixed.
I like that patch. Can this go in please?
Bump... can we get another look at this patch?
So what exactly is the holdup? I'd love to get the most security enabled by default. Why exactly are people not wanting security to be a priority?
Comment on attachment 216415 [details] [review] Only use SECURE256 if the GnuTLS version is less than 3 If there are going to be any divergences between gnutls 2 and gnutls 3 in the code, it should be in the direction of gnutls 3 being better, not worse. So this needs to be fixed to set up the priority strings correctly in gnutls 3 as well before I'd be willing to take it.
This code does 256 in gtls3 and does a best effort for 256 on gtls2. Why is that worse than forcing 128 bit on both? I am a bit mystified by your comment. Alternatively we could do some sort of api to generate the priority string. Forcing people to 128 bit encryption is not a good solution in my opinion. I am willing to burn some time on fixing this provided you can tell us how a patch would be acceptable.
no, the code does 256 in gnutls *2* and best-effort on 3.
Are you sure? The problem Xan was having with Twitter was that with the SECURE256 priority string was preventing the use of less secure cipher that Twitter required. On GnuTLS 2 this wasn't an issue because SECURE256 only tried the more secure NORMAL ciphers first instead of leaving them as a fallback. After looking more at the GnuTLS documentation for priority strings, it says it's possible to use '+' to append an algorithm. We should be able to use something like "SECURE256+less_secure_algorithms".
So I compiled the test program here[1] and ran it with NORMAL and SECURE256. I am now using GnuTLS 3.0.21. Here are the ciphers being used with NORMAL: Cipher suites for NORMAL TLS_ECDHE_ECDSA_AES_128_CBC_SHA1 0xc0, 0x09 TLS1.0 TLS_ECDHE_ECDSA_AES_128_CBC_SHA256 0xc0, 0x23 TLS1.2 TLS_ECDHE_ECDSA_AES_128_GCM_SHA256 0xc0, 0x2b TLS1.2 TLS_ECDHE_ECDSA_AES_256_CBC_SHA1 0xc0, 0x0a TLS1.0 TLS_ECDHE_ECDSA_AES_256_CBC_SHA384 0xc0, 0x24 TLS1.2 TLS_ECDHE_ECDSA_AES_256_GCM_SHA384 0xc0, 0x2c TLS1.2 TLS_ECDHE_ECDSA_3DES_EDE_CBC_SHA1 0xc0, 0x08 TLS1.0 TLS_ECDHE_RSA_AES_128_CBC_SHA1 0xc0, 0x13 TLS1.0 TLS_ECDHE_RSA_AES_128_CBC_SHA256 0xc0, 0x27 TLS1.2 TLS_ECDHE_RSA_AES_128_GCM_SHA256 0xc0, 0x2f TLS1.2 TLS_ECDHE_RSA_AES_256_CBC_SHA1 0xc0, 0x14 TLS1.0 TLS_ECDHE_RSA_AES_256_GCM_SHA384 0xc0, 0x30 TLS1.2 TLS_ECDHE_RSA_3DES_EDE_CBC_SHA1 0xc0, 0x12 TLS1.0 TLS_DHE_RSA_AES_128_CBC_SHA1 0x00, 0x33 SSL3.0 TLS_DHE_RSA_AES_128_CBC_SHA256 0x00, 0x67 TLS1.2 TLS_DHE_RSA_CAMELLIA_128_CBC_SHA1 0x00, 0x45 TLS1.0 TLS_DHE_RSA_AES_128_GCM_SHA256 0x00, 0x9e TLS1.2 TLS_DHE_RSA_AES_256_CBC_SHA1 0x00, 0x39 SSL3.0 TLS_DHE_RSA_AES_256_CBC_SHA256 0x00, 0x6b TLS1.2 TLS_DHE_RSA_CAMELLIA_256_CBC_SHA1 0x00, 0x88 TLS1.0 TLS_DHE_RSA_3DES_EDE_CBC_SHA1 0x00, 0x16 SSL3.0 TLS_DHE_DSS_AES_128_CBC_SHA1 0x00, 0x32 SSL3.0 TLS_DHE_DSS_AES_128_CBC_SHA256 0x00, 0x40 TLS1.2 TLS_DHE_DSS_CAMELLIA_128_CBC_SHA1 0x00, 0x44 TLS1.0 TLS_DHE_DSS_AES_128_GCM_SHA256 0x00, 0xa2 TLS1.2 TLS_DHE_DSS_AES_256_CBC_SHA1 0x00, 0x38 SSL3.0 TLS_DHE_DSS_AES_256_CBC_SHA256 0x00, 0x6a TLS1.2 TLS_DHE_DSS_CAMELLIA_256_CBC_SHA1 0x00, 0x87 TLS1.0 TLS_DHE_DSS_3DES_EDE_CBC_SHA1 0x00, 0x13 SSL3.0 TLS_DHE_DSS_ARCFOUR_SHA1 0x00, 0x66 TLS1.0 TLS_RSA_AES_128_CBC_SHA1 0x00, 0x2f SSL3.0 TLS_RSA_AES_128_CBC_SHA256 0x00, 0x3c TLS1.2 TLS_RSA_CAMELLIA_128_CBC_SHA1 0x00, 0x41 TLS1.0 TLS_RSA_AES_128_GCM_SHA256 0x00, 0x9c TLS1.2 TLS_RSA_AES_256_CBC_SHA1 0x00, 0x35 SSL3.0 TLS_RSA_AES_256_CBC_SHA256 0x00, 0x3d TLS1.2 TLS_RSA_CAMELLIA_256_CBC_SHA1 0x00, 0x84 TLS1.0 TLS_RSA_3DES_EDE_CBC_SHA1 0x00, 0x0a SSL3.0 TLS_RSA_ARCFOUR_SHA1 0x00, 0x05 SSL3.0 TLS_RSA_ARCFOUR_MD5 0x00, 0x04 SSL3.0 and again with SECURE256: Cipher suites for SECURE256 TLS_ECDHE_ECDSA_AES_256_CBC_SHA384 0xc0, 0x24 TLS1.2 TLS_ECDHE_ECDSA_AES_256_GCM_SHA384 0xc0, 0x2c TLS1.2 TLS_ECDHE_RSA_AES_256_GCM_SHA384 0xc0, 0x30 TLS1.2 TLS_DHE_RSA_AES_256_CBC_SHA256 0x00, 0x6b TLS1.2 TLS_DHE_DSS_AES_256_CBC_SHA256 0x00, 0x6a TLS1.2 TLS_RSA_AES_256_CBC_SHA256 0x00, 0x3d TLS1.2 [1] http://www.gnu.org/software/gnutls/manual/gnutls.html#Listing-the-ciphersuites-in-a-priority-string
For reference, here's the same output (using gnutls-cli, I should have used this too but whatever..) with GnuTLS 2.12.x: http://pastebin.com/MDnTg7jV
Created attachment 219653 [details] [review] Updated patch to prefer stronger crypto algorithms in GnuTLS 3.0 This patch still uses the SECURE256 priority string for GnuTLS versions less than 3.0, but uses a custom priority string (I called it NORMAL256) which uses the exact same cyphersuites as NORMAL but rearranges their priorities to prefer the more secure versions. The following shows the priorities of this NORMAL256 priority: Cipher suites for NORMAL:!CIPHER-ALL:+AES-256-GCM:+AES-256-CBC:+CAMELLIA-256-CBC:+AES-128-GCM:+AES-128-CBC:+CAMELLIA-128-CBC:+ARCFOUR-128:+3DES-CBC:!CURVE-ALL:+CURVE-SECP521R1:+CURVE-SECP384R1:+CURVE-SECP256R1:+CURVE-SECP224R1:+CURVE-SECP192R1:!SIGN-ALL:+SIGN-ECDSA-SHA512:+SIGN-ECDSA-SHA384:+SIGN-ECDSA-SHA256:+SIGN-ECDSA-SHA224:+SIGN-ECDSA-SHA1:+SIGN-RSA-SHA512:+SIGN-RSA-SHA384:+SIGN-RSA-SHA256:+SIGN-RSA-SHA224:+SIGN-RSA-SHA1:+SIGN-DSA-SHA256:+SIGN-DSA-SHA224:+SIGN-DSA-SHA1 TLS_ECDHE_ECDSA_AES_256_GCM_SHA384 0xc0, 0x2c TLS1.2 TLS_ECDHE_ECDSA_AES_256_CBC_SHA1 0xc0, 0x0a TLS1.0 TLS_ECDHE_ECDSA_AES_256_CBC_SHA384 0xc0, 0x24 TLS1.2 TLS_ECDHE_ECDSA_AES_128_GCM_SHA256 0xc0, 0x2b TLS1.2 TLS_ECDHE_ECDSA_AES_128_CBC_SHA1 0xc0, 0x09 TLS1.0 TLS_ECDHE_ECDSA_AES_128_CBC_SHA256 0xc0, 0x23 TLS1.2 TLS_ECDHE_ECDSA_3DES_EDE_CBC_SHA1 0xc0, 0x08 TLS1.0 TLS_ECDHE_RSA_AES_256_GCM_SHA384 0xc0, 0x30 TLS1.2 TLS_ECDHE_RSA_AES_256_CBC_SHA1 0xc0, 0x14 TLS1.0 TLS_ECDHE_RSA_AES_128_GCM_SHA256 0xc0, 0x2f TLS1.2 TLS_ECDHE_RSA_AES_128_CBC_SHA1 0xc0, 0x13 TLS1.0 TLS_ECDHE_RSA_AES_128_CBC_SHA256 0xc0, 0x27 TLS1.2 TLS_ECDHE_RSA_3DES_EDE_CBC_SHA1 0xc0, 0x12 TLS1.0 TLS_DHE_RSA_AES_256_CBC_SHA1 0x00, 0x39 SSL3.0 TLS_DHE_RSA_AES_256_CBC_SHA256 0x00, 0x6b TLS1.2 TLS_DHE_RSA_CAMELLIA_256_CBC_SHA1 0x00, 0x88 TLS1.0 TLS_DHE_RSA_AES_128_GCM_SHA256 0x00, 0x9e TLS1.2 TLS_DHE_RSA_AES_128_CBC_SHA1 0x00, 0x33 SSL3.0 TLS_DHE_RSA_AES_128_CBC_SHA256 0x00, 0x67 TLS1.2 TLS_DHE_RSA_CAMELLIA_128_CBC_SHA1 0x00, 0x45 TLS1.0 TLS_DHE_RSA_3DES_EDE_CBC_SHA1 0x00, 0x16 SSL3.0 TLS_DHE_DSS_AES_256_CBC_SHA1 0x00, 0x38 SSL3.0 TLS_DHE_DSS_AES_256_CBC_SHA256 0x00, 0x6a TLS1.2 TLS_DHE_DSS_CAMELLIA_256_CBC_SHA1 0x00, 0x87 TLS1.0 TLS_DHE_DSS_AES_128_GCM_SHA256 0x00, 0xa2 TLS1.2 TLS_DHE_DSS_AES_128_CBC_SHA1 0x00, 0x32 SSL3.0 TLS_DHE_DSS_AES_128_CBC_SHA256 0x00, 0x40 TLS1.2 TLS_DHE_DSS_CAMELLIA_128_CBC_SHA1 0x00, 0x44 TLS1.0 TLS_DHE_DSS_ARCFOUR_SHA1 0x00, 0x66 TLS1.0 TLS_DHE_DSS_3DES_EDE_CBC_SHA1 0x00, 0x13 SSL3.0 TLS_RSA_AES_256_CBC_SHA1 0x00, 0x35 SSL3.0 TLS_RSA_AES_256_CBC_SHA256 0x00, 0x3d TLS1.2 TLS_RSA_CAMELLIA_256_CBC_SHA1 0x00, 0x84 TLS1.0 TLS_RSA_AES_128_GCM_SHA256 0x00, 0x9c TLS1.2 TLS_RSA_AES_128_CBC_SHA1 0x00, 0x2f SSL3.0 TLS_RSA_AES_128_CBC_SHA256 0x00, 0x3c TLS1.2 TLS_RSA_CAMELLIA_128_CBC_SHA1 0x00, 0x41 TLS1.0 TLS_RSA_ARCFOUR_SHA1 0x00, 0x05 SSL3.0 TLS_RSA_ARCFOUR_MD5 0x00, 0x04 SSL3.0 TLS_RSA_3DES_EDE_CBC_SHA1 0x00, 0x0a SSL3.0 Certificate types: CTYPE-X.509 Protocols: VERS-TLS1.2, VERS-TLS1.1, VERS-TLS1.0, VERS-SSL3.0, VERS-DTLS1.0 Compression: COMP-NULL Elliptic curves: CURVE-SECP521R1, CURVE-SECP384R1, CURVE-SECP256R1, CURVE-SECP224R1, CURVE-SECP192R1 PK-signatures: SIGN-ECDSA-SHA512, SIGN-ECDSA-SHA384, SIGN-ECDSA-SHA256, SIGN-ECDSA-SHA224, SIGN-ECDSA-SHA1, SIGN-RSA-SHA512, SIGN-RSA-SHA384, SIGN-RSA-SHA256, SIGN-RSA-SHA224, SIGN-RSA-SHA1, SIGN-DSA-SHA256, SIGN-DSA-SHA224, SIGN-DSA-SHA1
Created attachment 219668 [details] [review] More GnuTLS 3.0 priority string fixes Here's an even better version of the last patch. This one also rearranges the MACs in a order more logical with the rest of the changes (most notable, AEAD has the highest MAC priority for use with the GCM AES ciphers). This change makes accounts.google.com now use the AES-256-GCM cipher instead of the less secure AES-128-CBC. Let me know if you find any other sites that seem like they should be using a more secure ciphersuite. If you can't find any, I think this patch should be ready for inclusion in the master branch.
Actually, my last comment is wrong. accounts.google.com is still defaulting to a 128-bit cipher with this priority string. I must have accidentally left out ARCFOUR-128 when testing earlier. Anyways, I also checked in Firefox (13.0.1) and Chrome (20.0.1132.57) and both seem to be defaulting to using ARCFOUR-128 on accounts.google.com. Anyways, the last patch is still the best as it uses the proper ordering for the MACs.
Comment on attachment 219668 [details] [review] More GnuTLS 3.0 priority string fixes >+#define NORMAL256 "NORMAL" \ >+ ":!CIPHER-ALL:+AES-256-GCM:+AES-256-CBC:+CAMELLIA-256-CBC:+AES-128-GCM:+AES-128-CBC:+CAMELLIA-128-CBC:+ARCFOUR-128:+3DES-CBC" \ >... The problem with this is that it's freezing the set of supported cryptosuites; when gnutls adds more, we won't get them because they're not in our string. And we can't just add new suites as gnutls supports them, because, eg, if gnutls 3.0.80 adds support for AES-1024-FOOBAR, and we add that to the string, it will stop working with gnutls 3.0.79 and earlier because the priority parser will reject unrecognized strings (which I guess is good for security reasons, to prevent typos from screwing you). Maybe we need to get some better gnutls API here. It seems like a bug that there's no easy way to do this... (Alternate acceptable patch idea: make it do g_getenv("G_TLS_GNUTLS_PRIORITY") and use that string if it's set [adjusting for rehandshake_mode and use_ssl3]. Then you guys can all set whatever priority string you want. Even better patch idea: implement gtls-related GSettings overrides, but 543455.)
Created attachment 219692 [details] [review] Optionally override the default "NORMAL:%COMPAT" priority string with the G_TLS_GNUTLS_PRIORITY environmental variable This patch grabs the G_TLS_GNUTLS_PRIORITY environmental variable and uses it to generate custom priority strings if the variable is unset.
Bump.. can we get another look at this patch?
This patch does what you expect right Dan? If so can we please get this in.
Comment on attachment 219692 [details] [review] Optionally override the default "NORMAL:%COMPAT" priority string with the G_TLS_GNUTLS_PRIORITY environmental variable I committed a patch based on that one. (Leaving the bug open since the original issue still applies.)
I don't know a lot about this stuff but I just wanted to make sure that with this change, we're still on the good side of all of the recent kerfuffle about attacks against SSL/TLS with block ciphers in CBC mode (BEAST, etc.). I know that other vendors were using this as justification for defaulting to stream ciphers for some time.
(In reply to comment #35) > I don't know a lot about this stuff but I just wanted to make sure that with > this change, we're still on the good side of all of the recent kerfuffle about > attacks against SSL/TLS with block ciphers in CBC mode (BEAST, etc.). The current state is that we use the gnutls defaults unless you set G_TLS_GNUTLS_PRIORITY. But you're right; if the user isn't explicitly trying to override them, we should keep gnutls's defaults rather than trying to come up with our own "better" ones, since gnutls is going to do a better job of adjusting its defaults to reflect new attacks than we would. (Though we should eventually also implement bug 543455, which would let a distro/administrator set new defaults system-wide if necessary.)