Move signature_algorithm extension to the end in NSS.

net/third_party/nss/ssl/ssl3ext.c

Index: net/third_party/nss/ssl/ssl3ext.c
diff --git a/net/third_party/nss/ssl/ssl3ext.c b/net/third_party/nss/ssl/ssl3ext.c
index 6f3fe2fa65191b6fd667c991e8c2c7df3381e063..72f5bdfc2408d3dd6a9acd3feb9025d7f884d23a 100644
--- a/net/third_party/nss/ssl/ssl3ext.c
+++ b/net/third_party/nss/ssl/ssl3ext.c
@@ -285,10 +285,6 @@ 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 },
@@ -297,7 +293,14 @@ ssl3HelloExtensionSender clientHelloSendersTLS[SSL_MAX_EXTENSIONS] = {
     { ssl_cert_status_xtn,        &ssl3_ClientSendStatusRequestXtn },
     { ssl_signature_algorithms_xtn, &ssl3_ClientSendSigAlgsXtn },
     { ssl_signed_certificate_timestamp_xtn,
-      &ssl3_ClientSendSignedCertTimestampXtn }
+      &ssl3_ClientSendSignedCertTimestampXtn },
+    /* WebSphere Application Server 7.0 is intolerant to the last extension
+     * being zero-length. ECC 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 },

[wtc, 2014/04/28 23:18:07]

Nit: please remove the trailing comma. An older version of the IBM compiler
doesn't allow that.

[davidben, 2014/04/28 23:35:02]

Done. Although it would get still be unhappy if NSS_ENABLE_ECC isn't defined
then. Should I also move the other trailing comma into the #ifdef?

[wtc, 2014/04/28 23:38:33]

I see. In that case I suggest moving ssl_server_name_xtn to the end. (In my own
patch for this bug, I just swap ssl_signature_algorithms_xtn and
ssl_signed_certificate_timestamp_xtn.)

[davidben, 2014/04/28 23:50:38]

Heh. Alright, I'll do that tomorrow and mirror it in the OpenSSL change. There
might be some small risk that servers expect SNI to be first or early; both
OpenSSL and NSS currently put it first. However, it looks like IE11 puts
server_name after renegotiation_info and Safari is renegotiation_info,
session_ticket, server_name. So there probably aren't servers that rely on it
being the very first.

[wtc, 2014/04/29 12:16:42]

I don't think this will be a problem. However, we can also just swap
ssl_signature_algorithms_xtn and ssl_signed_certificate_timestamp_xtn so that
ssl_signature_algorithms_xtn is the last entry. Although
ssl_signature_algorithms_xtn is only sent in TLS 1.2, WebSphere Application
Server/7.0 handles a TLS 1.2 ClientHello just fine, so
ssl_signature_algorithms_xtn should provide sufficient protection.

[davidben, 2014/04/30 20:49:37]

Alright, signature_algorithms then. :-P

+#endif
     /* any extra entries will appear as { 0, NULL }    */
 };
 
@@ -2347,9 +2350,11 @@ ssl3_CalculatePaddingExtensionLength(unsigned int clientHelloLength)
     }
 
     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. WebSphere Application Server
+     * 7.0 is intolerant to the last extension being zero-length. */
+    if (extensionLength < 4 + 1) {
+	extensionLength = 4 + 1;
     }
 
     return extensionLength;