[
https://bro-tracker.atlassian.net/browse/BIT-348?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16109#comment-16109
]
Jon Siwek commented on BIT-348:
-------------------------------
Addressed in topic/jsiwek/bit-348 branch in bro and bro-testing-private repos.
The test baselines change little so that's a good sign, however, I did end up
touching a decent amount of code which may make it better to postpone for a
later release. IMO, keeping it scheduled for 2.3 is probably the best way to
get the new coded tested, but I'm fine if others judge differently.
Also I think I kind of botched the commit message -- this change doesn't really
"fix" reassembly to deliver data after 2GB (which mostly worked before despite
the int overflows), it more just makes the reassembly not depend on "int" to be
32-bit and for "seq_delta" to provide correct sequence space arithmetic despite
overflows of that storage unit. That is, the reassembler code now deals in a
64-bit relative sequence space and it's up to the TCP analyzer to track the
32-bit sequence space and translate in to the 64-bit relative sequence space
when passing data on for reassembly.
Robin: good luck w/ the code review! I ended up refactoring more than I
probably needed to in TCP.cc, but I found it helpful to untangle some of what
was going on there.
> Reassembler integer overflow issues. Data not delivered after 2GB
> -----------------------------------------------------------------
>
> Key: BIT-348
> URL: https://bro-tracker.atlassian.net/browse/BIT-348
> Project: Bro Issue Tracker
> Issue Type: Problem
> Components: Bro
> Affects Versions: git/master
> Reporter: gregor
> Assignee: Jon Siwek
> Priority: High
> Labels: inttypes
> Fix For: 2.3
>
>
> {noformat}
> #!rst
> The TCP Reassembler does not deliver any data to analyzers after the first
> 2GB due to signed integer overflow (Actually it will deliver again between
> 4--6GB, etc.) This happens silently, i.e., without content_gap events or
> Undelivered calls.
> This report superseded BIT-315, BIT-137
> The TCP Reassembler (and Reassem) base class use ``int`` to keep track of
> sequence numbers and ``seq_delta`` to check for differences. If a connection
> exceeds 2GB, the relative sequence numbers (int) used by the Reassembler
> become negative. While many parts of the Reassembler still work (because
> seq_delta still reports the correct difference) some parts do not. In
> particular ``seq_to_skip`` is broken (and fails silently). There might well
> be other parts of the Reassembler that fail
> silently as well, that I haven't found yet.
> See Comments in TCP_Reassembler.cc for more details.
> The Reassembler should use int64. However this will require deep changes to
> the Reassembler and the TCP Analyzer and TCP_Endpoint classes (since we also
> store sequence numbers there). Also, the analyzer framework will need tweaks
> as well (e.g., Undelivered uses ``int`` for sequence numbers, also has to go
> to 64 bit)
> As a hotfix that seems to work I disabled the ``seq_to_skip`` features. It
> wasn't used by any analyzer or policy script (Note, that seq_to_skip is
> different from skip_deliveries). Hotfix is in
> topic/gregor/reassembler-hotfix
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.3-OD-02-026#6318)
_______________________________________________
bro-dev mailing list
[email protected]
http://mailman.icsi.berkeley.edu/mailman/listinfo/bro-dev
