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

Improvements to IP reassembly and TCP following #290

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

guyr
Copy link

@guyr guyr commented Apr 25, 2018

Hi Matias,

I have made some changes to libtins for my project, and I think most or all of them will be of value to the libtins community. The changes are as follows:

  • Modified src/ip_reassembler.cpp (and include/tins/ip_reassembler.h):

  • Modified src/tcp_ip/flow.cpp (function Flow::update_state) to correctly handle TCP session establishment when packets are (anomalously) duplicated.

  • Modified src/tcp_ip/stream.cpp (and include/tins/tcp_ip/stream.h):

    • Added an event (callback) of "connection establishment". (libtins originally only provided a callback for when a TCP stream is encountered, regardless of establishment).
    • Added Stream::ignore() to enable deletion of the stream from within TCP callbacks.
    • Added Stream::force_close() to force invocation of the connection-closed callback before deletion from outside of TCP callbacks.
  • Modified src/tcp_ip/stream_follower.cpp (and include/tins/tcp_ip/stream_follower.h):

    • Added StreamFollower::max_streams, a limitation on the number of concurrently-handled tcp connections. When the limit is reached, packets belonging to new connections are dropped.
      Zero means no limit.
    • Added StreamFollower::n_streams, a getter for the number of presently-followed streams.
    • Added StreamFollower::discard_stream. This invokes the connection termination callback.

I would like your opinion on which of these improvements you would like to pull, and whether any additional changes are required.
For sure at least two things still need to change:

  • The overlap support currently disregards the "technique" specified by the user and is always BSD. Please advise how you would like this implemented in libtins style.
  • For performance, the memory limit in ip_reassembler should use list instead of vector for the order of fragment streams. I am working on this.

Thanks,
Guy

@mfontanini mfontanini closed this May 10, 2018
@guyr
Copy link
Author

guyr commented May 10, 2018

Hi Matias,
Do I understand correctly that you are not interested in any of the changes?
Thanks,
Guy

@mfontanini mfontanini changed the base branch from develop to master May 10, 2018 16:28
@mfontanini mfontanini reopened this May 10, 2018
@mfontanini
Copy link
Owner

So sorry, this was closed automatically by github. I removed the develop branch (see this commit) which apparently closed this PR because it was using that branch as the base.

I really haven't had time to look at this. I have a backlog of PRs and issues I need to look into and this one's on my list. Thanks for the PR btw and sorry about the delay! I hope I can catch up with things in the next few days.

@guyr
Copy link
Author

guyr commented May 14, 2018

Cool, thanks :)

@guyr
Copy link
Author

guyr commented Jul 25, 2018

Hi Matias,

I made a performance improvement to the memory limitation mechanism in IPv4Reassembler.

Also, I see that there is a compilation error in OSX involving a name conflict with the constant for the BSD overlapping technique, so I guess a different name should be used for it. Please let me know how you would like it to be named.

Thanks,
Guy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants