Move signature_algorithm extension to the end in NSS.

net/third_party/nss/ssl/ssl3ext.c

 /*
  * SSL3 Protocol
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* TLS extension code moved here from ssl3ecc.c */
 
 #include "nssrenam.h"
@@ skipping 267 matching lines @@
 /* Tables of functions to format TLS hello extensions, one function per
  * extension.
  * These static tables are for the formatting of client hello extensions.
  * The server's table of hello senders is dynamic, in the socket struct,
  * and sender functions are registered there.
  */
 static const 
 ssl3HelloExtensionSender clientHelloSendersTLS[SSL_MAX_EXTENSIONS] = {
     { ssl_server_name_xtn,        &ssl3_SendServerNameXtn        },
     { ssl_renegotiation_info_xtn, &ssl3_SendRenegotiationInfoXtn },
-#ifdef NSS_ENABLE_ECC
-    { ssl_elliptic_curves_xtn,    &ssl3_SendSupportedCurvesXtn },
-    { ssl_ec_point_formats_xtn,   &ssl3_SendSupportedPointFormatsXtn },
-#endif
     { ssl_session_ticket_xtn,     &ssl3_SendSessionTicketXtn },
     { ssl_next_proto_nego_xtn,    &ssl3_ClientSendNextProtoNegoXtn },
     { ssl_app_layer_protocol_xtn, &ssl3_ClientSendAppProtoXtn },
     { ssl_use_srtp_xtn,           &ssl3_SendUseSRTPXtn },
     { ssl_channel_id_xtn,         &ssl3_ClientSendChannelIDXtn },
     { ssl_cert_status_xtn,        &ssl3_ClientSendStatusRequestXtn },
     { ssl_signature_algorithms_xtn, &ssl3_ClientSendSigAlgsXtn },
     { ssl_signed_certificate_timestamp_xtn,
-      &ssl3_ClientSendSignedCertTimestampXtn }
+      &ssl3_ClientSendSignedCertTimestampXtn },
+    /* Some servers are intolerant to the last extension being zero-length. ECC

[wtc, 2014/04/22 21:50:49]

Please replace "Some servers" with the name and version (if known) of the server
that has this bug, so that we will know when we can remove this workaround.

[davidben, 2014/04/28 19:52:26]

Done.

+     * extensions are non-empty and not dropped until fallback to SSL3, at which
+     * point all extensions are gone. */
+#ifdef NSS_ENABLE_ECC
+    { ssl_elliptic_curves_xtn,    &ssl3_SendSupportedCurvesXtn },
+    { ssl_ec_point_formats_xtn,   &ssl3_SendSupportedPointFormatsXtn },
+#endif
     /* any extra entries will appear as { 0, NULL }    */
 };
 
 static const 
 ssl3HelloExtensionSender clientHelloSendersSSL3[SSL_MAX_EXTENSIONS] = {
     { ssl_renegotiation_info_xtn, &ssl3_SendRenegotiationInfoXtn }
     /* any extra entries will appear as { 0, NULL }    */
 };
 
 static PRBool
@@ skipping 2029 matching lines @@
     unsigned int recordLength = 1 /* handshake message type */ +
                                 3 /* handshake message length */ +
                                 clientHelloLength;
     unsigned int extensionLength;
 
     if (recordLength < 256 || recordLength >= 512) {
         return 0;
     }
 
     extensionLength = 512 - recordLength;
-    /* Extensions take at least four bytes to encode. */
-    if (extensionLength < 4) {
-        extensionLength = 4;
+    /* Extensions take at least four bytes to encode. Always include at least
+     * one byte of data if including the extension. Some servers are intolerant
+     * to the last extension being empty. */

[wtc, 2014/04/22 21:50:49]

Good catch! I missed this in my workaround for this bug.

Nit: similarly we should name the server (and its version, if known) here.

+    if (extensionLength < 4 + 1) {
+        extensionLength = 4 + 1;
     }
 
     return extensionLength;
 }
 
 /* ssl3_AppendPaddingExtension possibly adds an extension which ensures that a
  * ClientHello record is either < 256 bytes or is >= 512 bytes. This ensures
  * that we don't trigger bugs in F5 products. */
 PRInt32
 ssl3_AppendPaddingExtension(sslSocket *ss, unsigned int extensionLen,
@@ skipping 77 matching lines @@
 
     if (!data->len) {
         /* Empty extension data: RFC 6962 mandates non-empty contents. */
         return SECFailure;
     }
     *scts = *data;
     /* Keep track of negotiated extensions. */
     ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
     return SECSuccess;
 }