Bug 946147: Move Move TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA ahead of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA to work around a broken server, r=wtc
authorBrian Smith <brian@briansmith.org>
Thu, 12 Dec 2013 22:30:49 -0800
changeset 10968 fda20ae7782440f8f638e5593d89695eda308bea
parent 10967 f6047eb1d0b840faea4d4c55a3af2ea9ea73a257
child 10969 6593411d388bf55c9f63f86e66aeff8eb75257f0
push id244
push userbrian@briansmith.org
push dateFri, 13 Dec 2013 06:31:24 +0000
reviewerswtc
bugs946147
Bug 946147: Move Move TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA ahead of TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA to work around a broken server, r=wtc
lib/ssl/ssl3con.c
lib/ssl/sslenum.c
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -77,28 +77,34 @@ static SECStatus ssl3_AESGCMBypass(ssl3K
 
 #define MAX_SEND_BUF_LENGTH 32000 /* watch for 16-bit integer overflow */
 #define MIN_SEND_BUF_LENGTH  4000
 
 /* This list of SSL3 cipher suites is sorted in descending order of
  * precedence (desirability).  It only includes cipher suites we implement.
  * This table is modified by SSL3_SetPolicy(). The ordering of cipher suites
  * in this table must match the ordering in SSL_ImplementedCiphers (sslenum.c)
+ *
+ * Important: See bug 946147 before enabling, reordering, or adding any cipher
+ * suites to this list.
  */
 static ssl3CipherSuiteCfg cipherSuites[ssl_V3_SUITES_IMPLEMENTED] = {
    /*      cipher_suite                     policy       enabled   isPresent */
 
 #ifdef NSS_ENABLE_ECC
  { TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,   SSL_ALLOWED, PR_FALSE, PR_FALSE},
+   /* TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA is out of order to work around
+    * bug 946147.
+    */
+ { TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,    SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,    SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,      SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,   SSL_ALLOWED, PR_FALSE, PR_FALSE},
- { TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,    SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,      SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA,   SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,     SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,        SSL_ALLOWED, PR_FALSE, PR_FALSE},
  { TLS_ECDHE_RSA_WITH_RC4_128_SHA,          SSL_ALLOWED, PR_FALSE, PR_FALSE},
 #endif /* NSS_ENABLE_ECC */
 
  { TLS_DHE_RSA_WITH_AES_128_GCM_SHA256,     SSL_ALLOWED, PR_TRUE,  PR_FALSE},
--- a/lib/ssl/sslenum.c
+++ b/lib/ssl/sslenum.c
@@ -29,26 +29,40 @@
  *      Camellia also has wide international support across standards
  *      organizations. SEED is only recommended by the Korean government. 3DES
  *      only provides 112 bits of security. RC4 is now deprecated or forbidden
  *      by many standards organizations.
  *    * Within symmetric algorithm sections, order by message authentication
  *      algorithm: GCM, then HMAC-SHA1, then HMAC-SHA256, then HMAC-MD5.
  *    * Within message authentication algorithm sections, order by asymmetric
  *      signature algorithm: ECDSA, then RSA, then DSS.
+ *
+ * Exception: Because some servers ignore the high-order byte of the cipher
+ * suite ID, we must be careful about adding cipher suites with IDs larger
+ * than 0x00ff; see bug 946147. For these broken servers, the first four cipher
+ * suites, with the MSB zeroed, look like:
+ *      TLS_KRB5_EXPORT_WITH_RC4_40_MD5 {0x00,0x2B }
+ *      TLS_RSA_WITH_AES_128_CBC_SHA { 0x00,0x2F }
+ *      TLS_RSA_WITH_3DES_EDE_CBC_SHA { 0x00,0x0A }
+ *      TLS_RSA_WITH_DES_CBC_SHA { 0x00,0x09 }
+ * The broken server only supports the third and fourth ones and will select
+ * the third one.
  */
 const PRUint16 SSL_ImplementedCiphers[] = {
 #ifdef NSS_ENABLE_ECC
     TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
     TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+    /* TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA must appear before
+     * TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA to work around bug 946147.
+     */
+    TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
     TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
     TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
     TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
     TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
-    TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
     TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
     TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA,
     TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
     TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
     TLS_ECDHE_RSA_WITH_RC4_128_SHA,
 #endif /* NSS_ENABLE_ECC */
 
     TLS_DHE_RSA_WITH_AES_128_GCM_SHA256,