Wireshark-dev: Re: [Wireshark-dev] [Outreachy] Multiple-line parsing of packets dissected over

From: Joey Salazar <jgsal@xxxxxxxxxxxxxx>
Date: Thu, 21 Jan 2021 20:36:58 +0000
Hi Pascal,
On Thursday, January 21, 2021 4:21 AM, Pascal Quantin <pascal@xxxxxxxxxxxxx> wrote:
Hi Joey,

Le mer. 20 janv. 2021 à 20:15, Joey Salazar <jgsal@xxxxxxxxxxxxxx> a écrit :
Hi Pascal,

On Wednesday, January 20, 2021 4:23 AM, Pascal Quantin wrote:

Hi Joey,

Unfortunately you did not share the associated TLS secret (or I missed it) that would allow me to decrypt the session and test your dissector. Could you send it?
My big apologies, I haven't worked with TLS certificates in the past and completely missed to send the secret. Apologies for taking your time. Please let me know if I'm missing anything else.

The use of a debugger clearly shows what the issue is:
- dissect_one_pkt_line() gets the length of the first line only with get_packet_length(). So the while loop after should be useless as you will consume the full line anyway as I stated previously
- dissect_one_pkt_line() is in fact intended to decode all lines, but you are not updating plen after adding the hf_git_packet_data item to the tree while incrementing offset. So you reuse the previous value of plen (that has been decremented by 4 after putting the hf_git_packet_len item), thus the value 0x0010 you get.
Your code should be instead something like this:

  total_len = tvb_reported_length(tvb);
  while (offset < total_len) {
    if (!get_packet_length(tvb, offset, &plen)) {
      /* XXX display expert info error? */
      return tvb_captured_length(tvb);
    }
    proto_tree_add_uint(git_tree, hf_git_packet_len, tvb, offset, 4, plen);
    offset += 4;
    plen -= 4;

    proto_tree_add_item(git_tree, hf_git_packet_data, tvb, offset, plen, ENC_NA);
    offset += plen;
    // To-do: add lines for parsing of terminator packet 0000
  }
  if (plen == 0) {
    proto_tree_add_uint(git_tree, hf_git_packet_terminator, tvb, offset,
                        4, plen);
  }
  return tvb_captured_length(tvb);

Note that in packet 169 there seems to be an issue with the fourth line that has a length of 1 only while you expect 4 at a minimum in your code. It needs to be properly handled.
This is most helpful and very appreciated, thank you. I think I have what I need for making progress.

Thanks,
Joey