New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tshark version problem #219

Closed
MorpheusDTA opened this Issue May 3, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@MorpheusDTA

MorpheusDTA commented May 3, 2017

Ostinato 0.8 does not work well when importing pcap files : with new versions of wireshark/tshark (2.2.6 for me), hex dump part for ip version & header length is changed from x45 to x04, triggering some alerts from wireshark when replayed.

Thanks for correcting it.

Original report and associated discussion on the mailing list

@ehlers

This comment has been minimized.

Show comment
Hide comment
@ehlers

ehlers Jun 26, 2017

Contributor

For testing IPv4 and IPv6 header issues when importing pcap and pdml files, I'm enclosing icmp_packets.zip with the following contents:

Archive:  icmp_packets.zip
  Length     Date   Time    Name
 --------    ----   ----    ----
      138  06-26-17 10:41   icmpv4.pcap
    10313  06-26-17 10:51   icmpv4_tshark1.pdml
    10226  06-26-17 10:43   icmpv4_tshark2.pdml
      110  06-26-17 10:42   icmpv6.pcap
    10152  06-26-17 10:51   icmpv6_tshark1.pdml
     9943  06-26-17 10:44   icmpv6_tshark2.pdml
 --------                   -------
    40882                   6 files

icmp_packets.zip

It uses a non-zero DSCP value and the don't fragment bit (on IPv4) to test also these fields.

The tshark1 files are pdml generated by tshark v1.12.1, the tshark2 files are generated by tshark v2.2.7.

I've noticed two issues with tshark v2. But I've tested only the IPv4 and IPv6 header, maybe there are more differences in other protocol headers:

IPv4: The ip.version field contains only the high 4 bits, while in tshark v1 it contains the whole byte (including the header length). A quick and dirty fix is to use the ip.hdr_len field. That contains the whole byte in both tshark versions. It's dirty, because in future this field may contain only the lower 4 bits (only the length bits).

Here the patch:

diff --git a/common/ip4pdml.cpp b/common/ip4pdml.cpp
index f355260..331ea72 100644
--- a/common/ip4pdml.cpp
+++ b/common/ip4pdml.cpp
@@ -26,7 +26,7 @@ PdmlIp4Protocol::PdmlIp4Protocol()
 {
     ostProtoId_ = OstProto::Protocol::kIp4FieldNumber;

-    fieldMap_.insert("ip.version", OstProto::Ip4::kVerHdrlenFieldNumber);
+    fieldMap_.insert("ip.hdr_len", OstProto::Ip4::kVerHdrlenFieldNumber);
     fieldMap_.insert("ip.dsfield", OstProto::Ip4::kTosFieldNumber);
     fieldMap_.insert("ip.len", OstProto::Ip4::kTotlenFieldNumber);
     fieldMap_.insert("ip.id", OstProto::Ip4::kIdFieldNumber);

IPv6: The traffic class field is named ipv6.class in tshark v1 and ipv6.tclass in tshark v2. My fix is to load both fields:

diff --git a/common/ip6pdml.cpp b/common/ip6pdml.cpp
index 2f3a7f8..b2fe70f 100644
--- a/common/ip6pdml.cpp
+++ b/common/ip6pdml.cpp
@@ -27,6 +27,7 @@ PdmlIp6Protocol::PdmlIp6Protocol()

     fieldMap_.insert("ipv6.version", OstProto::Ip6::kVersionFieldNumber);
     fieldMap_.insert("ipv6.class", OstProto::Ip6::kTrafficClassFieldNumber);
+    fieldMap_.insert("ipv6.tclass", OstProto::Ip6::kTrafficClassFieldNumber);
     fieldMap_.insert("ipv6.flow", OstProto::Ip6::kFlowLabelFieldNumber);
     fieldMap_.insert("ipv6.plen", OstProto::Ip6::kPayloadLengthFieldNumber);
     fieldMap_.insert("ipv6.nxt", OstProto::Ip6::kNextHeaderFieldNumber);
Contributor

ehlers commented Jun 26, 2017

For testing IPv4 and IPv6 header issues when importing pcap and pdml files, I'm enclosing icmp_packets.zip with the following contents:

Archive:  icmp_packets.zip
  Length     Date   Time    Name
 --------    ----   ----    ----
      138  06-26-17 10:41   icmpv4.pcap
    10313  06-26-17 10:51   icmpv4_tshark1.pdml
    10226  06-26-17 10:43   icmpv4_tshark2.pdml
      110  06-26-17 10:42   icmpv6.pcap
    10152  06-26-17 10:51   icmpv6_tshark1.pdml
     9943  06-26-17 10:44   icmpv6_tshark2.pdml
 --------                   -------
    40882                   6 files

icmp_packets.zip

It uses a non-zero DSCP value and the don't fragment bit (on IPv4) to test also these fields.

The tshark1 files are pdml generated by tshark v1.12.1, the tshark2 files are generated by tshark v2.2.7.

I've noticed two issues with tshark v2. But I've tested only the IPv4 and IPv6 header, maybe there are more differences in other protocol headers:

IPv4: The ip.version field contains only the high 4 bits, while in tshark v1 it contains the whole byte (including the header length). A quick and dirty fix is to use the ip.hdr_len field. That contains the whole byte in both tshark versions. It's dirty, because in future this field may contain only the lower 4 bits (only the length bits).

Here the patch:

diff --git a/common/ip4pdml.cpp b/common/ip4pdml.cpp
index f355260..331ea72 100644
--- a/common/ip4pdml.cpp
+++ b/common/ip4pdml.cpp
@@ -26,7 +26,7 @@ PdmlIp4Protocol::PdmlIp4Protocol()
 {
     ostProtoId_ = OstProto::Protocol::kIp4FieldNumber;

-    fieldMap_.insert("ip.version", OstProto::Ip4::kVerHdrlenFieldNumber);
+    fieldMap_.insert("ip.hdr_len", OstProto::Ip4::kVerHdrlenFieldNumber);
     fieldMap_.insert("ip.dsfield", OstProto::Ip4::kTosFieldNumber);
     fieldMap_.insert("ip.len", OstProto::Ip4::kTotlenFieldNumber);
     fieldMap_.insert("ip.id", OstProto::Ip4::kIdFieldNumber);

IPv6: The traffic class field is named ipv6.class in tshark v1 and ipv6.tclass in tshark v2. My fix is to load both fields:

diff --git a/common/ip6pdml.cpp b/common/ip6pdml.cpp
index 2f3a7f8..b2fe70f 100644
--- a/common/ip6pdml.cpp
+++ b/common/ip6pdml.cpp
@@ -27,6 +27,7 @@ PdmlIp6Protocol::PdmlIp6Protocol()

     fieldMap_.insert("ipv6.version", OstProto::Ip6::kVersionFieldNumber);
     fieldMap_.insert("ipv6.class", OstProto::Ip6::kTrafficClassFieldNumber);
+    fieldMap_.insert("ipv6.tclass", OstProto::Ip6::kTrafficClassFieldNumber);
     fieldMap_.insert("ipv6.flow", OstProto::Ip6::kFlowLabelFieldNumber);
     fieldMap_.insert("ipv6.plen", OstProto::Ip6::kPayloadLengthFieldNumber);
     fieldMap_.insert("ipv6.nxt", OstProto::Ip6::kNextHeaderFieldNumber);
@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jun 27, 2017

Owner

@ehlers Thanks for the proposed fixes. Do these fixes work with both Wireshark 1.x and 2.x?

Owner

pstavirs commented Jun 27, 2017

@ehlers Thanks for the proposed fixes. Do these fixes work with both Wireshark 1.x and 2.x?

@ehlers

This comment has been minimized.

Show comment
Hide comment
@ehlers

ehlers Jun 27, 2017

Contributor

That's the idea. But on my ostinato machine I have only Wireshark v2 installed. So I tested the PCAP and PDML import with Wireshark v2. Furthermore I loaded the PDML files from Wireshark v1 (from another machine) und checked, that it was correctly imported.

So you might want to check with your Ostinato and Wireshark v1 if the attached PCAP files are loading correctly.

Contributor

ehlers commented Jun 27, 2017

That's the idea. But on my ostinato machine I have only Wireshark v2 installed. So I tested the PCAP and PDML import with Wireshark v2. Furthermore I loaded the PDML files from Wireshark v1 (from another machine) und checked, that it was correctly imported.

So you might want to check with your Ostinato and Wireshark v1 if the attached PCAP files are loading correctly.

@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jun 27, 2017

Owner

@ehlers Will check on my machine and Wireshark v1. Meanwhile I'm trying to find out when and why was this change made in Wireshark to try and see if it affects more than just IPv4/IPv6.

Stay tuned. If you don't hear from me in a couple of days, please bump this ticket.

Owner

pstavirs commented Jun 27, 2017

@ehlers Will check on my machine and Wireshark v1. Meanwhile I'm trying to find out when and why was this change made in Wireshark to try and see if it affects more than just IPv4/IPv6.

Stay tuned. If you don't hear from me in a couple of days, please bump this ticket.

@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jun 28, 2017

Owner

Regarding IPv4 version and header length -

On June 18, 2014, IPv4 Version and Header Length were converted from byte value to bit values

On Apr 29, 2016, IPv4 Header Length was converted back to a byte value

This means some version(s) of Wireshark would work correctly and some won't. So it might be better for us to treat these two fields specially so that they work for all versions.

Regarding IPv6 traffic class -

As per the Wireshark IPv6 Display filter reference, ipv6.class is used in 1.x and ipv6.tclass in 2.x

Mapping both these Wireshark fields to the same Ostinato field is effectively a OR and hence the proposed fix should be ok.

I'm now trying to see if there are other fields that also changed between 1.x and 2.x and once I have that info, we can take a final call on how to proceed with the fix(es).

Stay tuned.

Owner

pstavirs commented Jun 28, 2017

Regarding IPv4 version and header length -

On June 18, 2014, IPv4 Version and Header Length were converted from byte value to bit values

On Apr 29, 2016, IPv4 Header Length was converted back to a byte value

This means some version(s) of Wireshark would work correctly and some won't. So it might be better for us to treat these two fields specially so that they work for all versions.

Regarding IPv6 traffic class -

As per the Wireshark IPv6 Display filter reference, ipv6.class is used in 1.x and ipv6.tclass in 2.x

Mapping both these Wireshark fields to the same Ostinato field is effectively a OR and hence the proposed fix should be ok.

I'm now trying to see if there are other fields that also changed between 1.x and 2.x and once I have that info, we can take a final call on how to proceed with the fix(es).

Stay tuned.

@ehlers

This comment has been minimized.

Show comment
Hide comment
@ehlers

ehlers Jun 28, 2017

Contributor

Regarding the IPv4 version and header length:

For me it looks, as if the complete byte can be retrieved from the unmaskedvalue attribute, when only some bits are included in the value.

So perhaps you might first try to use the unmaskedvalue attribute of ip.version. When that doesn't exist, use the value attribute of ip.version.

Contributor

ehlers commented Jun 28, 2017

Regarding the IPv4 version and header length:

For me it looks, as if the complete byte can be retrieved from the unmaskedvalue attribute, when only some bits are included in the value.

So perhaps you might first try to use the unmaskedvalue attribute of ip.version. When that doesn't exist, use the value attribute of ip.version.

@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jun 28, 2017

Owner

@ehlers That's right. I'm trying to determine if there are many more such changes (e.g. in other protocols) in which case I would try to make this change generically instead of specifically to IPv4.

Owner

pstavirs commented Jun 28, 2017

@ehlers That's right. I'm trying to determine if there are many more such changes (e.g. in other protocols) in which case I would try to make this change generically instead of specifically to IPv4.

@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jul 6, 2017

Owner

I'm not able to find the time to investigate which other fields may have broken with Wireshark 2.x, so I'm going to fix IPv4 and IPv6 for now and will raise a new bug as and when I find other fields which need a fix.

Owner

pstavirs commented Jul 6, 2017

I'm not able to find the time to investigate which other fields may have broken with Wireshark 2.x, so I'm going to fix IPv4 and IPv6 for now and will raise a new bug as and when I find other fields which need a fix.

@pstavirs pstavirs closed this in a8ec2f1 Jul 6, 2017

@pstavirs

This comment has been minimized.

Show comment
Hide comment
@pstavirs

pstavirs Jul 6, 2017

Owner

@ehlers Can you please verify this fix solves the problem with 2.x? I don't have access to that right now.

Owner

pstavirs commented Jul 6, 2017

@ehlers Can you please verify this fix solves the problem with 2.x? I don't have access to that right now.

@ehlers

This comment has been minimized.

Show comment
Hide comment
@ehlers

ehlers Jul 6, 2017

Contributor

Just verified it with Wireshark v2.2.1, works great. No issue found with IPv4, IPv6, ICMPv4 and ICMPv6.

Contributor

ehlers commented Jul 6, 2017

Just verified it with Wireshark v2.2.1, works great. No issue found with IPv4, IPv6, ICMPv4 and ICMPv6.

cdm-work pushed a commit to cdm-work/ostinato that referenced this issue Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment