ssl-utils: update supported_groups extension

Update references, rename the old "elliptic_curves" name to
"supported_groups". Fix a wrong field name (EC Point Format now has its
own hf). Add length validation for "elliptic_curve_list".

Change-Id: I554ebb259ba7561b48dfe1cc9162a0b3b3bcdba4
Reviewed-on: https://code.wireshark.org/review/20007
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

2 files changed, 66 insertions, 43 deletions

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 93df78bead..c33cde32af 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -423,6 +423,10 @@ static const value_string ssl_20_cipher_suites[] = {
 value_string_ext ssl_20_cipher_suites_ext = VALUE_STRING_EXT_INIT(ssl_20_cipher_suites);
 
 
+/*
+ * Supported Groups (formerly named "EC Named Curve").
+ * https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8
+ */
 const value_string ssl_extension_curves[] = {
     {  1, "sect163k1" },
     {  2, "sect163r1" },
@@ -454,11 +458,11 @@ const value_string ssl_extension_curves[] = {
     { 28, "brainpoolP512r1" }, /* RFC 7027 */
     { 29, "ecdh_x25519" }, /* https://tools.ietf.org/html/draft-ietf-tls-rfc4492bis */
     { 30, "ecdh_x448" }, /* https://tools.ietf.org/html/draft-ietf-tls-rfc4492bis */
-    { 256, "ffdhe2048" }, /* https://tools.ietf.org/html/draft-ietf-tls-negotiated-ff-dhe */
-    { 257, "ffdhe3072" }, /* https://tools.ietf.org/html/draft-ietf-tls-negotiated-ff-dhe */
-    { 258, "ffdhe4096" }, /* https://tools.ietf.org/html/draft-ietf-tls-negotiated-ff-dhe */
-    { 259, "ffdhe6144" }, /* https://tools.ietf.org/html/draft-ietf-tls-negotiated-ff-dhe */
-    { 260, "ffdhe8192" }, /* https://tools.ietf.org/html/draft-ietf-tls-negotiated-ff-dhe */
+    { 256, "ffdhe2048" }, /* RFC 7919 */
+    { 257, "ffdhe3072" }, /* RFC 7919 */
+    { 258, "ffdhe4096" }, /* RFC 7919 */
+    { 259, "ffdhe6144" }, /* RFC 7919 */
+    { 260, "ffdhe8192" }, /* RFC 7919 */
     { 0xFF01, "arbitrary_explicit_prime_curves" },
     { 0xFF02, "arbitrary_explicit_char2_curves" },
     { 0x00, NULL }
@@ -1156,7 +1160,7 @@ const value_string tls_hello_extension_types[] = {
     { SSL_HND_HELLO_EXT_CLIENT_AUTHZ, "client_authz" }, /* RFC 5878 */
     { SSL_HND_HELLO_EXT_SERVER_AUTHZ, "server_authz" }, /* RFC 5878 */
     { SSL_HND_HELLO_EXT_CERT_TYPE, "cert_type" }, /* RFC 6091 */
-    { SSL_HND_HELLO_EXT_SUPPORTED_GROUPS, "elliptic_curves" }, /* RFC 4492 */
+    { SSL_HND_HELLO_EXT_SUPPORTED_GROUPS, "supported_groups" }, /* RFC 4492, RFC 7919 */
     { SSL_HND_HELLO_EXT_EC_POINT_FORMATS, "ec_point_formats" }, /* RFC 4492 */
     { SSL_HND_HELLO_EXT_SRP, "srp" }, /* RFC 5054 */
     { SSL_HND_HELLO_EXT_SIGNATURE_ALGORITHMS, "signature_algorithms" }, /* RFC 5246 */
@@ -6514,35 +6518,48 @@ ssl_dissect_hnd_hello_ext_status_request_v2(ssl_common_dissect_t *hf, tvbuff_t *
     return offset;
 }
 
-static gint
-ssl_dissect_hnd_hello_ext_elliptic_curves(ssl_common_dissect_t *hf, tvbuff_t *tvb,
-                                          proto_tree *tree, guint32 offset)
+static guint
+ssl_dissect_hnd_hello_ext_supported_groups(ssl_common_dissect_t *hf, tvbuff_t *tvb, packet_info *pinfo,
+                                           proto_tree *tree, guint32 offset, guint32 offset_end)
 {
-    guint16     curves_length;
-    proto_tree *curves_tree;
+    /* https://tools.ietf.org/html/draft-ietf-tls-tls13-18#section-4.2.4
+     *  enum { ..., (0xFFFF) } NamedGroup;
+     *  struct {
+     *      NamedGroup named_group_list<2..2^16-1>
+     *  } NamedGroupList;
+     *
+     * NOTE: "NamedCurve" (RFC 4492) is renamed to "NamedGroup" (RFC 7919) and
+     * the extension itself from "elliptic_curves" to "supported_groups".
+     */
+    guint32     groups_length, next_offset;
+    proto_tree *groups_tree;
     proto_item *ti;
 
-    curves_length = tvb_get_ntohs(tvb, offset);
-    proto_tree_add_item(tree, hf->hf.hs_ext_elliptic_curves_len,
-                        tvb, offset, 2, ENC_BIG_ENDIAN);
-
+    /* NamedGroup named_group_list<2..2^16-1> */
+    if (!ssl_add_vector(hf, tvb, pinfo, tree, offset, offset_end, &groups_length,
+                        hf->hf.hs_ext_supported_groups_len, 2, G_MAXUINT16)) {
+        return offset_end;
+    }
     offset += 2;
+    next_offset = offset + groups_length;
+
     ti = proto_tree_add_none_format(tree,
-                                    hf->hf.hs_ext_elliptic_curves,
-                                    tvb, offset, curves_length,
-                                    "Elliptic curves (%d curve%s)",
-                                    curves_length / 2,
-                                    plurality(curves_length/2, "", "s"));
+                                    hf->hf.hs_ext_supported_groups,
+                                    tvb, offset, groups_length,
+                                    "Supported Groups (%d group%s)",
+                                    groups_length / 2,
+                                    plurality(groups_length/2, "", "s"));
 
     /* make this a subtree */
-    curves_tree = proto_item_add_subtree(ti, hf->ett.hs_ext_curves);
+    groups_tree = proto_item_add_subtree(ti, hf->ett.hs_ext_groups);
 
-    /* loop over all curves */
-    while (curves_length > 0)
-    {
-        proto_tree_add_item(curves_tree, hf->hf.hs_ext_elliptic_curve, tvb, offset, 2, ENC_BIG_ENDIAN);
+    /* loop over all groups */
+    while (offset + 2 <= offset_end) {
+        proto_tree_add_item(groups_tree, hf->hf.hs_ext_supported_group, tvb, offset, 2, ENC_BIG_ENDIAN);
         offset += 2;
-        curves_length -= 2;
+    }
+    if (!ssl_end_vector(hf, tvb, pinfo, groups_tree, offset, next_offset)) {
+        offset = next_offset;
     }
 
     return offset;
@@ -6562,7 +6579,7 @@ ssl_dissect_hnd_hello_ext_ec_point_formats(ssl_common_dissect_t *hf, tvbuff_t *t
 
     offset += 1;
     ti = proto_tree_add_none_format(tree,
-                                    hf->hf.hs_ext_elliptic_curves,
+                                    hf->hf.hs_ext_ec_point_formats,
                                     tvb, offset, ecpf_length,
                                     "Elliptic curves point formats (%d)",
                                     ecpf_length);
@@ -7467,7 +7484,7 @@ ssl_dissect_hnd_hello_ext(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *t
                 offset = ssl_dissect_hnd_hello_ext_status_request_v2(hf, tvb, ext_tree, offset);
             break;
         case SSL_HND_HELLO_EXT_SUPPORTED_GROUPS:
-            offset = ssl_dissect_hnd_hello_ext_elliptic_curves(hf, tvb, ext_tree, offset);
+            offset = ssl_dissect_hnd_hello_ext_supported_groups(hf, tvb, pinfo, ext_tree, offset, next_offset);
             break;
         case SSL_HND_HELLO_EXT_EC_POINT_FORMATS:
             offset = ssl_dissect_hnd_hello_ext_ec_point_formats(hf, tvb, ext_tree, offset);
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 93a045a967..fb51455d8f 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -148,7 +148,7 @@ typedef enum {
 #define SSL_HND_HELLO_EXT_CLIENT_AUTHZ                  7
 #define SSL_HND_HELLO_EXT_SERVER_AUTHZ                  8
 #define SSL_HND_HELLO_EXT_CERT_TYPE                     9
-#define SSL_HND_HELLO_EXT_SUPPORTED_GROUPS               10 /* renamed from "elliptic_curves (RFC7919)*/
+#define SSL_HND_HELLO_EXT_SUPPORTED_GROUPS              10 /* renamed from "elliptic_curves" (RFC 7919 / TLS 1.3) */
 #define SSL_HND_HELLO_EXT_EC_POINT_FORMATS              11
 #define SSL_HND_HELLO_EXT_SRP                           12
 #define SSL_HND_HELLO_EXT_SIGNATURE_ALGORITHMS          13
@@ -682,10 +682,11 @@ typedef struct ssl_common_dissect {
         gint hs_ext_cert_types_len;
         gint hs_ext_data;
         gint hs_ext_ec_point_format;
+        gint hs_ext_ec_point_formats;
         gint hs_ext_ec_point_formats_len;
-        gint hs_ext_elliptic_curve;
-        gint hs_ext_elliptic_curves;
-        gint hs_ext_elliptic_curves_len;
+        gint hs_ext_supported_group;
+        gint hs_ext_supported_groups;
+        gint hs_ext_supported_groups_len;
         gint hs_ext_heartbeat_mode;
         gint hs_ext_len;
         gint hs_ext_npn_str;
@@ -790,7 +791,7 @@ typedef struct ssl_common_dissect {
         gint hs_ext;
         gint hs_ext_alpn;
         gint hs_ext_cert_types;
-        gint hs_ext_curves;
+        gint hs_ext_groups;
         gint hs_ext_curves_point_formats;
         gint hs_ext_npn;
         gint hs_ext_reneg_info;
@@ -959,7 +960,7 @@ ssl_common_dissect_t name = {   \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
-        -1, -1, -1, -1, -1, -1, -1, -1, -1,                             \
+        -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,                         \
     },                                                                  \
     /* ett */ {                                                         \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
@@ -998,18 +999,18 @@ ssl_common_dissect_t name = {   \
         FT_BYTES, BASE_NONE, NULL, 0x0,                                 \
         "Hello Extension data", HFILL }                                 \
     },                                                                  \
-    { & name .hf.hs_ext_elliptic_curves_len,                            \
-      { "Elliptic Curves Length", prefix ".handshake.extensions_elliptic_curves_length",   \
+    { & name .hf.hs_ext_supported_groups_len,                           \
+      { "Supported Groups List Length", prefix ".handshake.extensions_supported_groups_length", \
         FT_UINT16, BASE_DEC, NULL, 0x0,                                 \
-        "Length of elliptic curves field", HFILL }                      \
+        NULL, HFILL }                                                   \
     },                                                                  \
-    { & name .hf.hs_ext_elliptic_curves,                                \
-      { "Elliptic Curves List", prefix ".handshake.extensions_elliptic_curves",        \
+    { & name .hf.hs_ext_supported_groups,                               \
+      { "Supported Groups List", prefix ".handshake.extensions_supported_groups", \
         FT_NONE, BASE_NONE, NULL, 0x0,                                  \
-        "List of elliptic curves supported", HFILL }                    \
+        "List of supported groups (formerly Supported Elliptic Curves)", HFILL } \
     },                                                                  \
-    { & name .hf.hs_ext_elliptic_curve,                                 \
-      { "Elliptic curve", prefix ".handshake.extensions_elliptic_curve",\
+    { & name .hf.hs_ext_supported_group,                                \
+      { "Supported Group", prefix ".handshake.extensions_supported_group", \
         FT_UINT16, BASE_HEX, VALS(ssl_extension_curves), 0x0,           \
         NULL, HFILL }                                                   \
     },                                                                  \
@@ -1018,6 +1019,11 @@ ssl_common_dissect_t name = {   \
         FT_UINT8, BASE_DEC, NULL, 0x0,                                  \
         "Length of elliptic curves point formats field", HFILL }        \
     },                                                                  \
+    { & name .hf.hs_ext_ec_point_formats,                               \
+      { "EC point formats", prefix ".handshake.extensions_ec_point_formats", \
+        FT_NONE, BASE_NONE, NULL, 0x0,                                  \
+        "List of elliptic curves point format", HFILL }                 \
+    },                                                                  \
     { & name .hf.hs_ext_ec_point_format,                                \
       { "EC point format", prefix ".handshake.extensions_ec_point_format",             \
         FT_UINT8, BASE_DEC, VALS(ssl_extension_ec_point_formats), 0x0,  \
@@ -1587,7 +1593,7 @@ ssl_common_dissect_t name = {   \
         & name .ett.hs_ext,                         \
         & name .ett.hs_ext_alpn,                    \
         & name .ett.hs_ext_cert_types,              \
-        & name .ett.hs_ext_curves,                  \
+        & name .ett.hs_ext_groups,                  \
         & name .ett.hs_ext_curves_point_formats,    \
         & name .ett.hs_ext_npn,                     \
         & name .ett.hs_ext_reneg_info,              \