SSL: bugfix for raw public keys in certificate message

RFC 7250 has changed the format of the Certificate structure from
RFC 5246 to the following:

opaque ASN.1Cert<1..2^24-1>;

struct {
    select(certificate_type) {

        // certificate type defined in RFC 7250
        case RawPublicKey:
           opaque ASN.1_subjectPublicKeyInfo<1..2^24-1>;

        // X.509 certificate defined in RFC 5246
        case X.509:
           ASN.1Cert certificate_list<0..2^24-1>;
    };
} Certificate;

Thus, ssl_dissect_hnd_cert() must parse subjectPublicKeyInfo
immediately when the message's certificate type is
SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY. Otherwise, the message will
contain a certificate_list.

This modification first determines the certificate type and then
handles both cases independently. For raw public keys, no subtree
is created to reflect the flat structure of the certificate
message.

Bug: 11480
Change-Id: I1c55eca361c4e40fcbff5bc32bfc8de3576bdfbf
Reviewed-on: https://code.wireshark.org/review/10272
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

1 files changed, 65 insertions, 31 deletions

diff --git a/epan/dissectors/packet-ssl-utils.c b/epan/dissectors/packet-ssl-utils.c
index e8db3eac14..fb5bce333f 100644
--- a/epan/dissectors/packet-ssl-utils.c
+++ b/epan/dissectors/packet-ssl-utils.c
@@ -5737,15 +5737,22 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
                      guint32 offset, packet_info *pinfo,
                      const SslSession *session, gint is_from_server)
 {
-    /* opaque ASN.1Cert<2^24-1>;
+    /* opaque ASN.1Cert<1..2^24-1>;
      *
      * struct {
-     *     ASN.1Cert certificate_list<1..2^24-1>;
+     *     select(certificate_type) {
+     *
+     *         // certificate type defined in RFC 7250
+     *         case RawPublicKey:
+     *           opaque ASN.1_subjectPublicKeyInfo<1..2^24-1>;
+     *
+     *         // X.509 certificate defined in RFC 5246
+     *         case X.509:
+     *           ASN.1Cert certificate_list<0..2^24-1>;
+     *     };
      * } Certificate;
      */
-    guint32     certificate_list_length;
-    proto_item *ti;
-    proto_tree *subtree;
+    enum { CERT_X509, CERT_RPK } cert_type;
     asn1_ctx_t  asn1_ctx;
 
     if (!tree)
@@ -5753,40 +5760,67 @@ ssl_dissect_hnd_cert(ssl_common_dissect_t *hf, tvbuff_t *tvb, proto_tree *tree,
 
     asn1_ctx_init(&asn1_ctx, ASN1_ENC_BER, TRUE, pinfo);
 
-    certificate_list_length = tvb_get_ntoh24(tvb, offset);
-    proto_tree_add_uint(tree, hf->hf.hs_certificates_len,
-                        tvb, offset, 3, certificate_list_length);
-    offset += 3;            /* 24-bit length value */
-
-    if (certificate_list_length > 0) {
-        ti = proto_tree_add_none_format(tree,
-                                        hf->hf.hs_certificates,
-                                        tvb, offset, certificate_list_length,
-                                        "Certificates (%u bytes)",
-                                        certificate_list_length);
-
-        /* make it a subtree */
-        subtree = proto_item_add_subtree(ti, hf->ett.certificates);
+    if ((is_from_server && session->server_cert_type == SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY) ||
+        (!is_from_server && session->client_cert_type == SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY)) {
+        cert_type = CERT_RPK;
+    } else {
+        cert_type = CERT_X509;
+    }
 
-        /* iterate through each certificate */
-        while (certificate_list_length > 0) {
-            /* get the length of the current certificate */
+    switch (cert_type) {
+    case CERT_RPK:
+        {
             guint32 cert_length;
             cert_length = tvb_get_ntoh24(tvb, offset);
-            certificate_list_length -= 3 + cert_length;
 
-            proto_tree_add_item(subtree, hf->hf.hs_certificate_len,
+            proto_tree_add_item(tree, hf->hf.hs_certificate_len,
                                 tvb, offset, 3, ENC_BIG_ENDIAN);
             offset += 3;
 
-            if ((is_from_server && session->server_cert_type == SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY) ||
-               (!is_from_server && session->client_cert_type == SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY)) {
-                dissect_x509af_SubjectPublicKeyInfo(FALSE, tvb, offset, &asn1_ctx, subtree, hf->hf.hs_certificate);
-            } else {
-                dissect_x509af_Certificate(FALSE, tvb, offset, &asn1_ctx, subtree, hf->hf.hs_certificate);
-            }
-
+            dissect_x509af_SubjectPublicKeyInfo(FALSE, tvb, offset, &asn1_ctx, tree, hf->hf.hs_certificate);
             offset += cert_length;
+
+            break;
+        }
+    case CERT_X509:
+        {
+            guint32     certificate_list_length;
+            proto_item *ti;
+            proto_tree *subtree;
+
+            certificate_list_length = tvb_get_ntoh24(tvb, offset);
+
+            proto_tree_add_uint(tree, hf->hf.hs_certificates_len,
+                                tvb, offset, 3, certificate_list_length);
+            offset += 3;            /* 24-bit length value */
+
+            if (certificate_list_length > 0) {
+                ti = proto_tree_add_none_format(tree,
+                                                hf->hf.hs_certificates,
+                                                tvb, offset, certificate_list_length,
+                                                "Certificates (%u bytes)",
+                                                certificate_list_length);
+
+                /* make it a subtree */
+                subtree = proto_item_add_subtree(ti, hf->ett.certificates);
+
+                /* iterate through each certificate */
+                while (certificate_list_length > 0) {
+                    /* get the length of the current certificate */
+                    guint32 cert_length;
+                    cert_length = tvb_get_ntoh24(tvb, offset);
+                    certificate_list_length -= 3 + cert_length;
+
+                    proto_tree_add_item(subtree, hf->hf.hs_certificate_len,
+                                        tvb, offset, 3, ENC_BIG_ENDIAN);
+                    offset += 3;
+
+                    dissect_x509af_Certificate(FALSE, tvb, offset, &asn1_ctx, subtree, hf->hf.hs_certificate);
+
+                    offset += cert_length;
+                }
+            }
+            break;
         }
     }
 }