Skip to content
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

[Bug] tcprewrite provides incorrect checksum for certain ipv4 packets #844

Closed
ChuckCottrill opened this issue Jan 24, 2024 · 5 comments
Closed
Assignees
Labels

Comments

@ChuckCottrill
Copy link
Contributor

ChuckCottrill commented Jan 24, 2024

The tcprewrite program provides incorrect checksum and modifies packet length in an undesireable manner.

Describe the bug
TCP rewrite produces an incorrect IP and TCP checksum for certain pcap files.
TCP rewrite appears to change packet length incorrectly, and thus produces an invalid checksum;
certain downstream processing may treat said incorrect checksum as a spoofing attempt and discard packet.

Expected behavior:
TCP rewrite should only change packet length when that behavior is specifically desired (command line option?).
TCP rewrite should correctly calculate IP and TCP checksum (incorrect because length incorrect).

To Reproduce

Steps to reproduce the behavior:

  1. uncompress packet captures:
mkdir -p pcaps
unzip tcprewrite-pcaps.zip
cp tcprewrite-pcaps/pcap-original-packet-3.pcap pcaps/.
  1. Run tcprewrite version 4.4.0 and observe the output, as follows
# version 4.4.0
VERSION="4.4.0"
# prepare
tcpreplay-4.4.0/src/tcpprep \
    --cidr=0.0.0.0/0 \
    --pcap=pcaps/pcap-original-packet-3.pcap \
    --cachefile=pcaps/pcap.cache
# use tcprewrite to rewrite packet addresses
tcpreplay-4.4.0/src/tcprewrite \
    --cachefile=pcaps/pcap.cache \
    --infile=pcaps/pcap-original-packet-3.pcap \
    --outfile=pcaps/cap-4.4.0-packet-out.pcap \
    --endpoints=10.200.1.1:10.200.1.2
  1. Run tcprewrite version 4.4.1 and observe the output, as follows
# version 4.4.1
VERSION="4.4.1"
# prepare
tcpreplay-4.4.1/src/tcpprep \
    --cidr=0.0.0.0/0 \
    --pcap=pcaps/pcap-original-packet-3.pcap \
    --cachefile=pcaps/pcap.cache
# use tcprewrite to rewrite packet addresses
tcpreplay-4.4.1/src/tcprewrite \
    --cachefile=pcaps/pcap.cache \
    --infile=pcaps/pcap-original-packet-3.pcap \
    --outfile=pcaps/cap-4.4.1-packet-out.pcap \
    --endpoints=10.200.1.1:10.200.1.2
  1. compare files, should be identical
bdiff pcaps/cap-4.4.0-packet-out.pcap pcaps/cap-4.4.1-packet-out.pcap

Packet Captures

Packet Captures to Reproduce:

  • pcap-original-packet-3.pcap
  • pcap-4.4.0-packet-3.pcap
  • pcap-4.4.1-packet-3.pcap

tcprewrite-pcaps.zip

Examine packets

Use Wireshark to examine and compare both packets.

  • Note that the ver 4.4.1 reports incorrect checksum.
  • Note also that packet length was changed, which is different behavior from desired.
  • Perhaps a flag to specify whether length change is needed or desired?

Screenshots
N/A - use Wireshark to view packets

System (please complete the following information):

  • OS: Linux
  • OS version
    • Linux hostname 5.15.0-71-generic #78-Ubuntu SMP datetime x86_64 x86_64 x86_64 GNU/Linux
  • Tcpreplay Version [4.4.1] versus [4.4.0]

Additional context
The erroneous checksum is due to the changed length. The problem results in dropped packet.

@ChuckCottrill
Copy link
Contributor Author

After looking deeper into the cause of this, the assumption which produced the problem, was that a user would always want to change (fix) the header length.
Users may only want to change (fix) the header length when desired. Thus a flag to turn on (default off) this behavior, would avoid surprises for those using 4.4.0, and moving to 4.4.1

@ChuckCottrill
Copy link
Contributor Author

This issue is related to issue #845
The issue resolves the problems encountered with modifying packet length.
This issue does not address the underlying quesion, what should be the packet length calculation result.

@fklassen fklassen self-assigned this Jun 1, 2024
@fklassen fklassen added the bug label Jun 1, 2024
@fklassen
Copy link
Member

fklassen commented Jun 1, 2024

This issue does not address the underlying quesion, what should be the packet length calculation result.

I would like to mark this issue resolved by PR #843. I'm a bit concerned about this statement. Can you elaborate? Can a new ticket be opened if the packet length calculation is incorrect (other than issue #845)?

@fklassen
Copy link
Member

fklassen commented Jun 1, 2024

Verified PR #846 fixes this:

~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯ src/tcprewrite --cachefile=pcaps/pcap.cache \
    --infile=pcaps/cap-original-packet-3.pcap \
    --outfile=pcaps/cap-4.5.0-packet-out.pcap \
    --endpoints=10.200.1.1:10.200.1.2
Warning in ../../src/tcprewrite.c:post_args() line 231:
pcaps/cap-original-packet-3.pcap was captured using a snaplen of 1514 bytes.  This may mean you have truncated packets.
~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯ diff pcaps/cap-4.4.0-packet-3.pcap pcaps/cap-4.5.0-packet-out.pcap
~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯

fklassen added a commit that referenced this issue Jun 1, 2024
added to control action on packet length changes

Verification

```
~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯ src/tcprewrite --cachefile=pcaps/pcap.cache \
    --infile=pcaps/cap-original-packet-3.pcap \
    --outfile=pcaps/cap-4.5.0-packet-out.pcap \
    --endpoints=10.200.1.1:10.200.1.2
Warning in ../../src/tcprewrite.c:post_args() line 231:
pcaps/cap-original-packet-3.pcap was captured using a snaplen of 1514 bytes.  This may mean you have truncated packets.
~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯ diff pcaps/cap-4.4.0-packet-3.pcap pcaps/cap-4.5.0-packet-out.pcap
~/git/tcpreplay/build Bug_703_844_PR_846_optionally_fix_pkt_hdr_len* ⇡
❯
```
fklassen added a commit that referenced this issue Jun 1, 2024
Triggers when attempting a checksum update but packet length does not match header length.

Example:

```
tcprewrite -i test.pcap -o test2.rewrite_1ttl --ttl=58
Warning: skipping packet 11 because caplen 122 minus L2 length 22 does not equal IPv4 header length 255. Consider option '--fixhdrlen'.
Warning: skipping packet 12 because caplen 114 minus L2 length 14 does not equal IPv4 header length 84. Consider option '--fixhdrlen'.
Warning: skipping packet 14 because caplen 60 minus L2 length 14 does not equal IPv4 header length 28. Consider option '--fixhdrlen'.
Warning: skipping packet 15 because caplen 60 minus L2 length 14 does not equal IPv4 header length 28. Consider option '--fixhdrlen'.
Warning: skipping packet 16 because caplen 60 minus L2 length 14 does not equal IPv4 header length 28. Consider option '--fixhdrlen'.
```
fklassen added a commit that referenced this issue Jun 2, 2024
Required to run tests on older bigendian machine.

Also fix a usec to nsec conversion bug.
fklassen added a commit that referenced this issue Jun 2, 2024
include tests for --fixhdrlen option
fklassen added a commit that referenced this issue Jun 2, 2024
…x_pkt_hdr_len

Bug #703 #844 PR #846: optionally fix packet header length --fixhdrlen
fklassen added a commit that referenced this issue Jun 2, 2024
libpcap nanosec values are reported in tv_usec.
fklassen added a commit that referenced this issue Jun 2, 2024
libpcap nanosec values are reported in tv_usec.

Not so for legacy libpcap which has usec values in tv_usec.
fklassen added a commit that referenced this issue Jun 2, 2024
fklassen added a commit that referenced this issue Jun 2, 2024
libpcap nanosec values are reported in tv_usec.

Not so for legacy libpcap which has usec values in tv_usec.
fklassen added a commit that referenced this issue Jun 2, 2024
@fklassen
Copy link
Member

fklassen commented Jun 2, 2024

Fixed in PR #846

@fklassen fklassen closed this as completed Jun 2, 2024
fklassen added a commit that referenced this issue Jun 8, 2024
This ioctl error suggests that the tap interface has already been set.

Backed out #411 and #651 as they only partially address the issue and introduced new bugs.
fklassen added a commit that referenced this issue Jun 8, 2024
…il_intermittently

Bug #844 tap: ignore TUNSETIFF EBUSY errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants