ssl,dtls: fix CertificateVerify dissection for TLSv1.2

TLSv1.2 gained an additional SignatureAndHashAlgorithm field for fields
marked with the digitally-signed attribute. This was already implemented
before for ServerKeyExchange, let's reuse that.

Note that the SignatureAndHashAlgorithm tree and fields (hash algo,
signature algo) are repurposed in a different context, but since the
structure is the same it is kept like this.

By the way, add support for DTLSv1.2 too. RFC 6347 section 4.2.6
suggests that the implementation is the same (as far as the dissector is
concerned).

Also update the comments and remove the additional "Signature with
client's private key" subtree since the CertificateVerify message has no
other items.

Bug: 11045
Change-Id: I025901b85e607f04d60357ff14187cc13db2ae5d
Reviewed-on: https://code.wireshark.org/review/7650
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Michael Mann <mmann78@netscape.net>

4 files changed, 70 insertions, 65 deletions

diff --git a/epan/dissectors/packet-dtls.c b/epan/dissectors/packet-dtls.c
index 12e14d434d..2c9f770cea 100644
--- a/epan/dissectors/packet-dtls.c
+++ b/epan/dissectors/packet-dtls.c
@@ -1381,7 +1381,7 @@ dissect_dtls_handshake(tvbuff_t *tvb, packet_info *pinfo,
             break;
 
           case SSL_HND_CERT_VERIFY:
-            /* unimplemented */
+            ssl_dissect_hnd_cli_cert_verify(&dissect_dtls_hf, tvb, ssl_hand_tree, offset, session);
             break;
 
           case SSL_HND_CLIENT_KEY_EXCHG:
diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index 34450578de..61955910fe 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -5821,6 +5821,22 @@ ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb,
     }
 }
 
+static void
+ssl_dissect_digitally_signed(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                             proto_tree *tree, guint32 offset,
+                             const SslSession *session,
+                             gint hf_sig_len, gint hf_sig);
+
+void
+ssl_dissect_hnd_cli_cert_verify(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                                proto_tree *tree, guint32 offset,
+                                const SslSession *session)
+{
+    ssl_dissect_digitally_signed(hf, tvb, tree, offset, session,
+                                 hf->hf.hs_client_cert_vrfy_sig_len,
+                                 hf->hf.hs_client_cert_vrfy_sig);
+}
+
 void
 ssl_dissect_hnd_finished(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                          proto_tree *tree, guint32 offset,
@@ -6285,13 +6301,12 @@ dissect_ssl3_hnd_cli_keyex_rsa_psk(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 }
 
 
-/* ServerKeyExchange algo-specific dissectors */
-
-/* dissects signed_params inside a ServerKeyExchange for some keyex algos */
+/* Dissects DigitallySigned (see RFC 5246 4.7 Cryptographic Attributes). */
 static void
-dissect_ssl3_hnd_srv_keyex_sig(ssl_common_dissect_t *hf, tvbuff_t *tvb,
-                               proto_tree *tree, guint32 offset,
-                               const SslSession *session)
+ssl_dissect_digitally_signed(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                             proto_tree *tree, guint32 offset,
+                             const SslSession *session,
+                             gint hf_sig_len, gint hf_sig)
 {
     gint        sig_len;
     proto_item *ti_algo;
@@ -6318,10 +6333,35 @@ dissect_ssl3_hnd_srv_keyex_sig(ssl_common_dissect_t *hf, tvbuff_t *tvb,
 
     /* Sig */
     sig_len = tvb_get_ntohs(tvb, offset);
-    proto_tree_add_item(tree, hf->hf.hs_server_keyex_sig_len, tvb,
-                        offset, 2, ENC_BIG_ENDIAN);
-    proto_tree_add_item(tree, hf->hf.hs_server_keyex_sig, tvb,
-                        offset + 2, sig_len, ENC_NA);
+    proto_tree_add_item(tree, hf_sig_len, tvb, offset, 2, ENC_BIG_ENDIAN);
+    proto_tree_add_item(tree, hf_sig, tvb, offset + 2, sig_len, ENC_NA);
+}
+
+/* ServerKeyExchange algo-specific dissectors */
+
+/* dissects signed_params inside a ServerKeyExchange for some keyex algos */
+static void
+dissect_ssl3_hnd_srv_keyex_sig(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                               proto_tree *tree, guint32 offset,
+                               const SslSession *session)
+{
+    /*
+     * TLSv1.2 (RFC 5246 sec 7.4.8)
+     *  struct {
+     *      digitally-signed struct {
+     *          opaque handshake_messages[handshake_messages_length];
+     *      }
+     *  } CertificateVerify;
+     *
+     * TLSv1.0/TLSv1.1 (RFC 5436 sec 7.4.8 and 7.4.3) works essentially the same
+     * as TLSv1.2, but the hash algorithms are not explicit in digitally-signed.
+     *
+     * SSLv3 (RFC 6101 sec 5.6.8) esseentially works the same as TLSv1.0 but it
+     * does more hashing including the master secret and padding.
+     */
+    ssl_dissect_digitally_signed(hf, tvb, tree, offset, session,
+                                 hf->hf.hs_server_keyex_sig_len,
+                                 hf->hf.hs_server_keyex_sig);
 }
 
 static void
diff --git a/epan/dissectors/packet-ssl-utils.h b/epan/dissectors/packet-ssl-utils.h
index 19fcf2c26f..6a4242f5a8 100644
--- a/epan/dissectors/packet-ssl-utils.h
+++ b/epan/dissectors/packet-ssl-utils.h
@@ -727,6 +727,8 @@ typedef struct ssl_common_dissect {
         gint hs_session_ticket_len;
         gint hs_session_ticket;
         gint hs_finished;
+        gint hs_client_cert_vrfy_sig_len;
+        gint hs_client_cert_vrfy_sig;
 
         /* do not forget to update SSL_COMMON_LIST_T and SSL_COMMON_HF_LIST! */
     } hf;
@@ -808,6 +810,11 @@ ssl_dissect_hnd_cert_req(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                           const SslSession *session);
 
 extern void
+ssl_dissect_hnd_cli_cert_verify(ssl_common_dissect_t *hf, tvbuff_t *tvb,
+                                proto_tree *tree, guint32 offset,
+                                const SslSession *session);
+
+extern void
 ssl_dissect_hnd_finished(ssl_common_dissect_t *hf, tvbuff_t *tvb,
                          proto_tree *tree, guint32 offset,
                          const SslSession *session, ssl_hfs_t *ssl_hfs);
@@ -835,6 +842,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,                                                         \
     },                                                                  \
     /* ett */ {                                                         \
         -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, \
@@ -1328,6 +1336,16 @@ ssl_common_dissect_t name = {   \
       { "Verify Data", prefix ".handshake.verify_data",                 \
         FT_NONE, BASE_NONE, NULL, 0x0,                                  \
         "Opaque verification data", HFILL }                             \
+    },                                                                  \
+    { & name .hf.hs_client_cert_vrfy_sig_len,                           \
+      { "Signature length", prefix ".handshake.client_cert_vrfy.sig_len", \
+        FT_UINT16, BASE_DEC, NULL, 0x0,                                 \
+        "Length of CertificateVerify's signature", HFILL }              \
+    },                                                                  \
+    { & name .hf.hs_client_cert_vrfy_sig,                               \
+      { "Signature", prefix ".handshake.client_cert_vrfy.sig",          \
+        FT_BYTES, BASE_NONE, NULL, 0x0,                                 \
+        "CertificateVerify's signature", HFILL }                        \
     }
 /* }}} */
 
diff --git a/epan/dissectors/packet-ssl.c b/epan/dissectors/packet-ssl.c
index c956605249..593026d33f 100644
--- a/epan/dissectors/packet-ssl.c
+++ b/epan/dissectors/packet-ssl.c
@@ -133,8 +133,6 @@ static gint hf_ssl_alert_message_description  = -1;
 static gint hf_ssl_handshake_protocol         = -1;
 static gint hf_ssl_handshake_type             = -1;
 static gint hf_ssl_handshake_length           = -1;
-static gint hf_ssl_handshake_client_cert_vrfy_sig_len = -1;
-static gint hf_ssl_handshake_client_cert_vrfy_sig     = -1;
 static gint hf_ssl_handshake_cert_status      = -1;
 static gint hf_ssl_handshake_cert_status_type = -1;
 static gint hf_ssl_handshake_cert_status_len  = -1;
@@ -196,7 +194,6 @@ static gint ett_ssl_alert             = -1;
 static gint ett_ssl_handshake         = -1;
 static gint ett_ssl_heartbeat         = -1;
 static gint ett_ssl_certs             = -1;
-static gint ett_ssl_cli_sig           = -1;
 static gint ett_ssl_cert_status       = -1;
 static gint ett_ssl_ocsp_resp         = -1;
 static gint ett_pct_cipher_suites     = -1;
@@ -454,10 +451,6 @@ static void dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo,
                                    const SslSession *session, guint32 record_length,
                                    gboolean decrypted);
 
-static void dissect_ssl3_hnd_cli_cert_verify(tvbuff_t *tvb,
-                                            proto_tree *tree,
-                                            guint32 offset, guint32 length);
-
 static void dissect_ssl3_hnd_cert_status(tvbuff_t *tvb,
                                          proto_tree *tree,
                                          guint32 offset,
@@ -1989,7 +1982,7 @@ dissect_ssl3_handshake(tvbuff_t *tvb, packet_info *pinfo,
                 break;
 
             case SSL_HND_CERT_VERIFY:
-                dissect_ssl3_hnd_cli_cert_verify(tvb, ssl_hand_tree, offset, length);
+                ssl_dissect_hnd_cli_cert_verify(&dissect_ssl3_hf, tvb, ssl_hand_tree, offset, session);
                 break;
 
             case SSL_HND_CLIENT_KEY_EXCHG:
@@ -2128,41 +2121,6 @@ dissect_ssl3_heartbeat(tvbuff_t *tvb, packet_info *pinfo,
     }
 }
 
-static void
-dissect_ssl3_hnd_cli_cert_verify(tvbuff_t *tvb, proto_tree *tree,
-                                guint32 offset, guint32 length)
-{
-    proto_tree *ssl_sig_tree;
-
-    /*
-       struct {
-          Signature signature;
-       } CertificateVerify;
-
-       Signature is a digitally-signed struct {...}, depending on the algorithm
-
-       "A digitally-signed element is encoded as an opaque
-        vector <0..2^16-1>, where the length is specified by the
-        signing algorithm and key."
-
-        <> is a variable length vector. It starts with a length field
-        large enough to encode the largest possible length.
-
-        -> The signature starts with a two-byte length field.
-     */
-
-
-    ssl_sig_tree = proto_tree_add_subtree(tree, tvb, offset, length,
-            ett_ssl_cli_sig, NULL, "Signature with client's private key");
-
-    proto_tree_add_item(ssl_sig_tree, hf_ssl_handshake_client_cert_vrfy_sig_len,
-                        tvb, offset, 2, ENC_BIG_ENDIAN);
-    /* XXX check that ..._vrfy_sig_len == length-2 */
-    proto_tree_add_item(ssl_sig_tree, hf_ssl_handshake_client_cert_vrfy_sig,
-                        tvb, offset+2, length-2, ENC_NA);
-}
-
-
 static guint
 dissect_ssl3_ocsp_response(tvbuff_t *tvb, proto_tree *tree,
                            guint32 offset, packet_info *pinfo)
@@ -3768,16 +3726,6 @@ proto_register_ssl(void)
             FT_UINT24, BASE_HEX|BASE_EXT_STRING, &ssl_20_cipher_suites_ext, 0x0,
             "Cipher specification", HFILL }
         },
-        { &hf_ssl_handshake_client_cert_vrfy_sig_len,
-            { "Signature length", "ssl.handshake.client_cert_vrfy.sig_len",
-              FT_UINT16, BASE_DEC, NULL, 0x0,
-              "Length of CertificateVerify's signature", HFILL }
-        },
-        { &hf_ssl_handshake_client_cert_vrfy_sig,
-            { "Signature", "ssl.handshake.client_cert_vrfy.sig",
-              FT_BYTES, BASE_NONE, NULL, 0x0,
-              "CertificateVerify's signature", HFILL }
-        },
         { &hf_ssl_handshake_cert_status,
           { "Certificate Status", "ssl.handshake.cert_status",
             FT_NONE, BASE_NONE, NULL, 0x0,
@@ -4041,7 +3989,6 @@ proto_register_ssl(void)
         &ett_ssl_handshake,
         &ett_ssl_heartbeat,
         &ett_ssl_certs,
-        &ett_ssl_cli_sig,
         &ett_ssl_cert_status,
         &ett_ssl_ocsp_resp,
         &ett_pct_cipher_suites,