Fix length calculation for GTPv2 header #3833
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3833 +/- ##
==========================================
- Coverage 49.95% 49.94% -0.02%
==========================================
Files 228 228
Lines 53176 53176
==========================================
- Hits 26565 26559 -6
- Misses 26611 26617 +6
|
| @@ -258,7 +258,7 @@ class GTPHeader(Packet): | |||
| def post_build(self, p, pay): | |||
| p += pay | |||
| if self.length is None: | |||
| tmp_len = len(p) - 8 | |||
| tmp_len = len(p) - 4 if self.version == 2 else len(p) - 8 | |||
There was a problem hiding this comment.
Could you explain how a test on the version fixes your issue?
There was a problem hiding this comment.
Absolutely! That field is defined differently for GTPv1 Headers and GTPv2 Headers. See for instance https://en.wikipedia.org/wiki/GPRS_Tunnelling_Protocol#Header
For GTPv1:
Message Length
a 16-bit field that indicates the length of the payload in bytes (rest of the packet following the mandatory 8-byte GTP header). Includes the optional fields.
For GTPv2:
Message length
This field shall indicate the length of the message in octets excluding the mandatory of the GTP-C header (the first 4 octets).
So for GTPv1 you need to disregard 8 bytes, whereas for GTPv2 it's only 4. The GTPHeader class was correctly implementing the length calculation for GTPv1, but not for GTPv2.
I could also change the PR and overwrite this entire method in the gtp_v2.GTPHeader, which so far inherits all the logic from the gtp.GTPHeader: https://github.com/secdev/scapy/blob/master/scapy/contrib/gtp_v2.py#L242-L264
Now that I write this, I think that this is a much better place for this logic...
There was a problem hiding this comment.
That's not necessary, thanks a lot for clarifying ! Would mind adding a very short (1-2lines) summary of this as a comment ?
Thanks !
3GPP TS 29.274 states in Section 5.5.1: "Octets 3 to 4 represent the Message Length field. This field shall indicate the length of the message in octets excluding the mandatory part of the GTP-C header (the first 4 octets). The TEID (if present) and the Sequence Number shall be included in the length count."
* Fix length calculation for GTPv2 header 3GPP TS 29.274 states in Section 5.5.1: "Octets 3 to 4 represent the Message Length field. This field shall indicate the length of the message in octets excluding the mandatory part of the GTP-C header (the first 4 octets). The TEID (if present) and the Sequence Number shall be included in the length count." * Add comment on length in different GTP versions --------- Co-authored-by: Mike Müller <mmuller@twilio.com>
3GPP TS 29.274 states in Section 5.5.1:
"Octets 3 to 4 represent the Message Length field. This field shall indicate the length of the message in octets excluding the mandatory part of the GTP-C header (the first 4 octets). The TEID (if present) and the Sequence Number shall be included in the length count."
Checklist:
cd test && ./run_testsortox)