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

Fix/overlapping tcp segments #1898

Merged
merged 3 commits into from
Jun 27, 2016
Merged

Conversation

urso
Copy link

@urso urso commented Jun 22, 2016

No description provided.

delta := lastSeq - tcpStartSeq

// if 'overlap' already covered by previous packet -> return
if int(delta) >= len(pkt.Payload) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add at least a debug log message here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug message is ok. but this is perfectly valid behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rethinking this case, it's impossible to trigger this condition due to tcpSeq already compared to lastSeq and if reset, packet is dropped.

@ruflin
Copy link
Member

ruflin commented Jun 22, 2016

LGTM. Worth backporting?

@urso
Copy link
Author

urso commented Jun 22, 2016

Yeah. in presence of overlapping resends, application layer parser are likely to trip or generate panic.

@ruflin ruflin added the needs_backport PR is waiting to be backported to other branches. label Jun 22, 2016
@ruflin
Copy link
Member

ruflin commented Jun 22, 2016

I added the backport layer. There seem to be quite some packetbeat system tests that are failing ...

@urso urso force-pushed the fix/overlapping-tcp-segments branch from 31634fb to 18ebe01 Compare June 22, 2016 16:34
@urso urso mentioned this pull request Jun 23, 2016
@urso urso force-pushed the fix/overlapping-tcp-segments branch from 18ebe01 to 5401d12 Compare June 23, 2016 13:00
@urso urso force-pushed the fix/overlapping-tcp-segments branch from 5401d12 to c44d735 Compare June 27, 2016 10:16
@tsg tsg merged commit 391e840 into elastic:master Jun 27, 2016
urso pushed a commit to urso/beats that referenced this pull request Jun 27, 2016
* TCP table driven unit test
* Fix tcp segments overlaps
tsg pushed a commit that referenced this pull request Jul 19, 2016
* TCP table driven unit test
* Fix tcp segments overlaps
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Sep 22, 2016
@urso urso deleted the fix/overlapping-tcp-segments branch February 19, 2019 18:31
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
* TCP table driven unit test
* Fix tcp segments overlaps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants