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

tcpprep with pcap containing >1020 packets produces invalid cache file #415

Closed
petegallagher opened this issue Aug 5, 2017 · 5 comments
Closed
Assignees
Labels
Projects

Comments

@petegallagher
Copy link

petegallagher commented Aug 5, 2017

Version: 4.2.5

Steps to reproduce

  1. Using a pcap file containing >1020 packets e.g.:

    curl http://packetlife.net/captures/snmp-ipv4.cap --output tcpreplay_issue_415.pcap
    
  2. Call tcpprep using auto-split mode e.g.:

    tcpprep --auto=router --cachefile=tcpreplay_issue_415.cache --pcap=tcpreplay_issue_415.pcap
    
  3. Call tcprewrite using cache file above e.g.:

    tcprewrite --endpoints=192.168.0.1:192.168.1.1 \
               --cachefile=tcpreplay_issue_415.cache \
               --infile=tcpreplay_issue_415.pcap \
               --outfile=tcpreplay_issue_415_rewritten.pcap
    
  4. Error is displayed on console e.g.:

    Fatal Error in cache.c:read_cache() line 138:
     Cache data length (256 bytes) doesn't match cache header (525 bytes)
    

Description

When running tcpprep with a large pcap file (my example is ~4GB, containing 5763149 packets) it appears as though the cachedata packet count (e.g. mycache->packets) is only ever a maximum of 1020, subsequently only 256 bytes (1020/4=255) of cachedata is written to the cache file. This ultimately causes the following error when the cache file is used later (e.g. with tcprewrite):

Fatal Error: Cache data length (256 bytes) doesn't match cache header (1440788 bytes)

From the debugging I've done so far and my rudimentary understanding of the code, I believe the cause of the issue is that cachedata packet count is not correctly incremented once we begin to use linked lists of caches (see common/cache.c:276).

It seems the first cachedata item has its packet count correctly incremented up to the maximum 1020. However for each subsequent packet (>1020), a new cache is created and appended to the linked list, and this cache contains a single packet only. This means that if you have 1024 packets, you would have 5 caches in the linked list. This can be observed by running with --dbug=1 e.g. see below for the debug logs showing the packet counts for each cache:

$ grep "Cache array packet" tcpprep-debug.log | sort | uniq -c
5762130 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 1
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 2
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 3
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 4
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 5
.
. edited for clarity
.
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 1018
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 1019
      1 DEBUG1 in cache.c:add_cache() line 288: Cache array packet 1020

My example pcap contains 5763149 packets. The output above shows that the first cache is continuously incremented up to 1020, subsequently we have 5762129 caches created with a single packet each (5762129+1020=5763149).

So there appears to be 2 defects here:

  1. The total packet count is not being incremented correctly. This causes knock on effects when trying to write the cachedata to a file.
  2. The linked caches are not being filled to their capacity (i.e. it would seem the initial count is being used for all subsequent new caches)
@petegallagher petegallagher changed the title tcpprep with large cache causes mismatch with header tcpprep with pcap containing >1020 packets produces invalid cache file Aug 5, 2017
@fklassen fklassen self-assigned this Aug 8, 2017
@fklassen fklassen added the bug label Aug 8, 2017
@petegallagher
Copy link
Author

As a workaround I adjusted the CACHEDATASIZE in common/cache.h to be larger than my total number of packets divided by 4. For example 5763149 / 4 = 1440787.25, so my CACHEDATASIZE=1450000. If I recompile with this value then everything works as expected with my sample capture.

@fklassen
Copy link
Member

@petegallagher Thanks for digging into this. I plan to finalize a fix for next release which will probably be before the end of the month.

@z2xy
Copy link

z2xy commented Oct 29, 2017

seems has't been fixed... @fklassen

@fklassen
Copy link
Member

Yeah, life caught up to me. I will take some time off in December to work on this project.

@fklassen fklassen added this to To Do in 4.3 Jan 7, 2018
@fklassen fklassen moved this from To Do to In progress in 4.3 Jan 21, 2018
fklassen added a commit that referenced this issue Jan 21, 2018
fklassen added a commit that referenced this issue Jan 21, 2018
@fklassen
Copy link
Member

Fixed in #426

4.3 automation moved this from In progress to Done Jan 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
4.2.7
Awaiting triage
4.3
  
Done
Development

No branches or pull requests

3 participants