Hi Joey, Le mer. 20 janv. 2021 à 20:15, Joey Salazar <[email protected]> a écrit :
> Hi Pascal, > > On Wednesday, January 20, 2021 4:23 AM, Pascal Quantin wrote: > > Hi Joey, > > Le mar. 19 janv. 2021 à 23:35, Joey Salazar <[email protected]> a > écrit : > >> On Tuesday, January 19, 2021 4:20 PM, Pascal Quantin wrote: >> >> Le mar. 19 janv. 2021 à 23:09, Joey Salazar <[email protected]> a >> écrit : >> >>> Hi Pascal, >>> On Tuesday, January 19, 2021 11:19 AM, Pascal Quantin wrote: >>> >>> Hi Joey, >>> >>> Le mar. 19 janv. 2021 à 17:45, Joey Salazar via Wireshark-dev < >>> [email protected]> a écrit : >>> >>>> 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? >>>> >>> >>> So what is the code leading to this dissection? It does not seem to be >>> gitlab.com/joeysal/wireshark/-/commit/33af2649927cb5660d4aeb64b9a9e9a58a1823aa >>> as dissect_one_pkt_line() seem to read only one line >>> >>> 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.
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
