Hi Pascal,
On Wednesday, January 20, 2021 4:23 AM, Pascal Quantin wrote:
Hi Joey,
On Tuesday, January 19, 2021 4:20 PM, Pascal Quantin wrote:
Hi Pascal,
On Tuesday, January 19, 2021 11:19 AM, Pascal Quantin wrote:
Hi Joey,
Hi all,
In commit 33af2649 [1] we can keep dissecting the contents of the req, adv, and res packets by setting
while (plen > 0) { }
either in `dissect_git_pdu()` or in `dissect_one_pkt_line()`, but for now in `dissect_git_pdu()` it'd be a bit messy, so wanted to ask for your feedback for getting `dissect_one_pkt_line()` to work properly first.
As you can see in pcap 169 [2], it correctly parses the length of the first line as 0x0014 (20 bytes) until `0x0a`, then it's supposed to get the length of the next line by the first 4 hex bytes in that line, but instead of reading the length as 0x0018 (24 bytes) it's reading it as 0x0010 (16 bytes), and anyways, this particular line's length actually is 59 bytes.
Suggestions on how to approach this?
Yes, the code on that commit is what gives the parsing of the screenshot.
So what mechanism is used to call dissect_one_plt_line() a second time? With only screenshots and no pcap / code to look at, we can hardly help.
The code has already been provided. I confirm again that there hasn't been other lines added other than what's in that commit.
Does it mean that packet-http.c calls your dissector per line? Please provide more info, or even better share the pcap if you want us to provide some help.
Please find attached the pcap I'm using with the patch from the commit. As you can see, the way 167 and 255 are parsed is similar, but I'm referring specifically to 169 for now ("To-do" in line 121 will be for the cases where there's a 0000 terminator packet like the end of the first-line in 167) .
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.
Hope this helps,
Pascal.
