[Bro-Dev] #861: Merging DNP3 Analyzer

[Bro Tracker]

#861: Merging DNP3 Analyzer
---------------------+------------------------
  Reporter:  hui     |      Owner:  robin
      Type:  Task    |     Status:  assigned
  Priority:  Normal  |  Milestone:  Bro2.2
 Component:  Bro     |    Version:  git/master
Resolution:          |   Keywords:  dnp3
---------------------+------------------------

Comment (by hui):

 Hi, Robin,

 I need to resolve a confusion related to global variable you mentioned in
 the email. Other comments are being resolved.

 The DNP 3 trace that I am using usually follow this pattern
 for short packets:
 TCP handshake
 DNP3 request
 DNP3 response
 TCP Close Session
 (For this situation, I don't need the global variable in my codes, a local
 variable will do)

 Howerver, for longer packets
 TCP handshake
 DNP3 packet #1
 DNP3 packet #2
 ....
 DNP3 response
 TCP

 Well this looks similar as a single HTTP fragment spanned into several TCP
 payloads. I am sure that TCP reassembly in Bro will do that. Though I am
 not sure how TCP reassembly works, my intuition is the length of payload,
 or other fields to indicate that this is not the last application payload.
 But things are getting ugly for DNP3 packets. In details, it looks like
 this:

 TCP : DNP3 Serial Link Layer : DNP3 Serial Transport Layer : DNP3 Serial
 Application Layer #1 (292 bytes)
 TCP : DNP3 Serial Link Layer : DNP3 Serial Transport Layer : DNP3 Serial
 Application Layer #2 (292 bytes)
 TCP : DNP3 Serial Link Layer : DNP3 Serial Transport Layer : DNP3 Serial
 Application Layer #3
 ...

 So what are concatenated are the DNP3 Serial Application Layer #1 + DNP3
 Serial Application Layer #2 + DNP3 Serial Application Layer #3 +
 ....Instead of concatenating application layer payload such as HTTP. In
 this case, I use a global variable to across different flow to do this
 job. All these flows are within a single TCP session, so I guess that is
 why it is working. Fortunately, this long DNP3 packet is very rare, but I
 do have test pcap.

 So my understanding is that a analyzer class instance represent a single
 TCP session including all flows ? Actually I am writing to confirm this
 understanding.

 Hope this helps, let me know if there is any question related to this
 issue.

 Hui

 Replying to [comment:3 robin]:
 > I've merged this with master into the temporary branch
 {{topic/topic/robin/dnp3-merge}}.
 >
 > Hui, a number of points/questions:
 >
 > - I've added a set of {{{TODO-Hui}}} throughout the new code. Please
 take a look and address (just grep for it).
 >
 > - I moved the global variables in {{{DNP3.cc}{} into the analyzer class.
 I'm actually surprised that this has ever worked: it looks like you kept
 state across flows with a single variable; am I missing something?
 >
 > - Please take a look at the new test
 {{{scripts.base.protocols.dnp3.events}}} and check the output if it
 matches with what you would expect.
 >
 > - Which of the many events did you have data for to test with? I've
 added the DNP3 traces from Dina, they trigger 11 of the 51 events. Do we
 have more we can add to the test suite?
 >
 > - Please document the events in {{{src/events.bif}}}, similar to how
 other events are documented.
 >
 > - Does DNP3 have cases similar to Modbus where it would make sense to
 pass arrays of integers (or other elements)? If so, that would be good to
 do (but I don't know the protocol enough to say more).

-- 
Ticket URL: <http://tracker.bro-ids.org/bro/ticket/861#comment:6>
Bro Tracker <http://tracker.bro-ids.org/bro>
Bro Issue Tracker