Reorder extensions to put SigAlgs last
authorTodd Short <tshort@akamai.com>
Thu, 13 Jul 2017 14:47:16 +0000 (10:47 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Wed, 16 Aug 2017 16:58:00 +0000 (11:58 -0500)
WebSphere application server cannot handle having an empty
extension (e.g. EMS/EtM) as the last extension in a client hello.
This moves the SigAlgs extension last (before any padding) for TLSv1.2
to avoid this issue.

Force the padding extension to a minimum length of 1.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3927)

ssl/t1_lib.c

index 8cd0b965444e292cdcd4d339cf3cfe942ef5b81b..55abba9619427a3d5f098b5a9bdeb34cf439026b 100644 (file)
@@ -1197,30 +1197,6 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
     }
  skip_ext:
 
-    if (SSL_CLIENT_USE_SIGALGS(s)) {
-        size_t salglen;
-        const unsigned char *salg;
-        unsigned char *etmp;
-        salglen = tls12_get_psigalgs(s, 1, &salg);
-
-        /*-
-         * check for enough space.
-         * 4 bytes for the sigalgs type and extension length
-         * 2 bytes for the sigalg list length
-         * + sigalg list length
-         */
-        if (CHECKLEN(ret, salglen + 6, limit))
-            return NULL;
-        s2n(TLSEXT_TYPE_signature_algorithms, ret);
-        etmp = ret;
-        /* Skip over lengths for now */
-        ret += 4;
-        salglen = tls12_copy_sigalgs(s, ret, salg, salglen);
-        /* Fill in lengths */
-        s2n(salglen + 2, etmp);
-        s2n(salglen, etmp);
-        ret += salglen;
-    }
 #ifndef OPENSSL_NO_OCSP
     if (s->tlsext_status_type == TLSEXT_STATUSTYPE_ocsp) {
         int i;
@@ -1415,11 +1391,42 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
     s2n(TLSEXT_TYPE_extended_master_secret, ret);
     s2n(0, ret);
 
+    /*
+     * WebSphere application server can not handle having the
+     * last extension be 0-length (e.g. EMS, EtM), so keep those
+     * before SigAlgs
+     */
+    if (SSL_CLIENT_USE_SIGALGS(s)) {
+        size_t salglen;
+        const unsigned char *salg;
+        unsigned char *etmp;
+        salglen = tls12_get_psigalgs(s, 1, &salg);
+
+        /*-
+         * check for enough space.
+         * 4 bytes for the sigalgs type and extension length
+         * 2 bytes for the sigalg list length
+         * + sigalg list length
+         */
+        if (CHECKLEN(ret, salglen + 6, limit))
+            return NULL;
+        s2n(TLSEXT_TYPE_signature_algorithms, ret);
+        etmp = ret;
+        /* Skip over lengths for now */
+        ret += 4;
+        salglen = tls12_copy_sigalgs(s, ret, salg, salglen);
+        /* Fill in lengths */
+        s2n(salglen + 2, etmp);
+        s2n(salglen, etmp);
+        ret += salglen;
+    }
+
     /*
      * Add padding to workaround bugs in F5 terminators. See
      * https://tools.ietf.org/html/draft-agl-tls-padding-03 NB: because this
      * code works out the length of all existing extensions it MUST always
-     * appear last.
+     * appear last. WebSphere 7.x/8.x is intolerant of empty extensions
+     * being last, so minimum length of 1.
      */
     if (s->options & SSL_OP_TLSEXT_PADDING) {
         int hlen = ret - (unsigned char *)s->init_buf->data;
@@ -1429,7 +1436,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *buf,
             if (hlen >= 4)
                 hlen -= 4;
             else
-                hlen = 0;
+                hlen = 1;
 
             /*-
              * check for enough space. Strictly speaking we know we've already