Wireshark mailing list archives
Re: [Wireshark-commits] rev 46239: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-sctp.c
From: Michael Tuexen <Michael.Tuexen () micmac franken de>
Date: Thu, 29 Nov 2012 23:30:41 +0100
On Nov 29, 2012, at 8:32 PM, Jeff Morriss wrote:
Michael Tuexen wrote:On Nov 28, 2012, at 12:13 AM, morriss () wireshark org wrote:http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=46239 User: morriss Date: 2012/11/27 03:13 PM Log: Warn about non-final parameters that aren't padded correctly. I'm not sure that final parameters *with* padding is all that important (maybe it should be there but not PI_ERROR?).Sorry, I keep forgetting to respond...
No problem...
Do you have an example where the non-final parameter is not padded correctly? I though this can't happen, so I took the expert warning out. But possibly I missed a case.What do you mean it "can't happen?" Of course an implementation could fail to pad the parameter correctly. In fact I thought I saw one which is why I put the expert info in. Though in the end it turned out that it *was* the last parameter.
I think when the dissector breaks the chunk into parameters, it will always try to get the padding, which is possible for all parameters except for the last. And the last should not contain the padding. That is why I took out the code and replaced it by checking if the last one contains padding. The change committed in http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=46290 is now pretty defensive. It covers both cases.
RFC 4960 says regarding the chunk length: The Chunk Length value does not include terminating padding of the chunk. However, it does include padding of any variable-length parameter except the last parameter in the chunk. The receiver MUST ignore the padding. Note: A robust implementation should accept the chunk whether or not the final padding has been included in the Chunk Length. So I changed the code to make the "except the last parameter in the chunk" visible. At least this made me aware of a bug in the FreeBSD implementation. So we should have such an expert info.I have to admit that exception confused me greatly. I was going to ask the tsvwg about it. I'm not quite sure I see the point. And of course there's no key word (MUST, SHOULD) to tell us how important is the exception. My presumption would be not many implementations would blow up if the exception were not taken into account--thus my comment about maybe including the expert info but not making it PI_ERROR. PI_NOTE maybe?
FreeBSD doesn't follow the RFC currently. I'm fixing this right now... I don't think the level makes much of a difference. I was surprised that FreeBSD doesn't do it correctly, so I really want the expert warning to find also other problems and fix it. Without being aware of it, it wouldn't be fixed. Best regards Michael
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Re: [Wireshark-commits] rev 46239: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-sctp.c Jeff Morriss (Nov 29)
- Re: [Wireshark-commits] rev 46239: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-sctp.c Michael Tuexen (Nov 30)