Re: Hui Lin_DNP3 analyzer not working in current version of zeek

Hi Jon,

This is a very confusing part of DNP3 protocol, but only in the request.
The "number_of_item" obtained from "object_header" in the request is mainly
applied to the response, telling outstation that this is the number of
items that you should include in the response. But there are a few
exceptions where the request is also using the "number_of_item", especially
the control operations, meaning that these are the operations that we would
like to apply to the outstation.

Back then when I had implemented it, I use a common record
type, Request_Data_Object, to hide those differences. In the READ request,
there still should be Request_Data_Object[8]. However, according to the
value of the function code (which is the input of the Request_Data_Object),
the size of each Request_Data_Object becomes 0 if it is READ request. Even
though there are 8 objects, but each of them has 0 bytes, so totally there
are no data coming. That is why actually there are no more data coming,
which is not a mistake. I guess that it is probably how binpac changes the
way to handle this type of situation now, making the analyzer fail to work.

For me, the workaround should not be difficult (also I am pretty sure that
Robin and Seth would like to keep the binpac works as it is now).  As there
are only a very few exceptions, I just include those exceptions directly
there (similar to what you did in the patch), and then default most other
requests to empty objects.

I probably will do that on July as I am catching a deadline on July 1st. I
will contact you privately as I may need you to help me to input a
different public key for upload as I have already graduated.

Best,

Hui Lin

On Mon, Jun 17, 2019 at 7:57 PM Jon Siwek <[email protected]> wrote:

> Found the difference:
>
> This behavior changed in Bro 2.5.4 (really what changed is in BinPAC
> 0.49, not Bro), but the new parsing behavior is legitimate.  The old
> behavior just caused broken protocol grammars to possibly parse more
> things than they should have, such as in cases where there wasn't enough
> data to fill an array.  So it appeared to be working for you, but it
> was not.
>
> In this case, the DNP3 protocol grammar we use is either incomplete or
> needs a further fix.  With some debugging for this example pcap, you can
> see where the parsing is failing:
>
> protocol violation, [orig_h=10.0.0.3, orig_p=37147/tcp,
> resp_h=10.0.0.1, resp_p=20000/tcp], Analyzer::ANALYZER_DNP3_TCP,
> Binpac exception: binpac exception: out_of_bound:
> Request_Objects:ojbects: 8 > 0
> protocol violation, [orig_h=10.0.0.3, orig_p=55021/tcp,
> resp_h=10.0.0.2, resp_p=20000/tcp], Analyzer::ANALYZER_DNP3_TCP,
> Binpac exception: binpac exception: out_of_bound:
> Request_Objects:ojbects: 8 > 0
>
> That's here:
>
>
> https://github.com/zeek/zeek/blob/e2dc0092f3a1caea1ebc71e347663e723298fb6b/src/analyzer/protocol/dnp3/dnp3-protocol.pac#L97
>
> You can look in the pcap (e.g. Wireshark) and see in the first READ
> request that there's no objects being sent for us to parse even though
> our protocol definition is written to expect that.  So that protocol
> violation is legit in the sense that we've defined the protocol in a way
> that differs from what's being sent on the wire.  And a protocol
> violation in the case of DNP3 disables all further analysis.
>
> You maybe understand DNP3 better than me, so please create an issue
> or pull request if you come up with a fix that improves the DNP3 parser.
> Attached is a naive patch that seems to generate the same number of
> requests/responses as before Bro 2.5.4; maybe it helps as a
> starting point or reference.
>
> - Jon
>
> On Mon, Jun 17, 2019 at 11:53 AM Hui Lin (Hugo) <[email protected]>
> wrote:
> >
> > Thanks Jon,
> >
> > The version that is working is version 2.5-457. I have attached the
> sample pcap here.
> >
> > Best,
> >
> > Hui Lin
> >
> > On Mon, Jun 17, 2019 at 10:32 AM Jon Siwek <[email protected]> wrote:
> >>
> >> On Sun, Jun 16, 2019 at 3:17 PM Hui Lin (Hugo) <[email protected]>
> wrote:
> >> > Actually for the same pcap, in a version that I git last year, bro
> works fine by printing all messages. Any idea what happens? If needed, I
> can provide the pcap for the testing.
> >>
> >> Not sure, please either try to debug / explore the diffs, or provide a
> >> pcap and say which was the last known working version.
> >>
> >> - Jon
> >
> >
> >
> > --
> > Hui Lin
> > Ph.D. Candidate (http://hlin33.web.engr.illinois.edu/)
> > DEPEND (http://depend.csl.illinois.edu/)
> > ECE, Uni. of Illinois at Urbana-Champaign
> >
> _______________________________________________
> Zeek mailing list
> [email protected]
> http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/zeek



-- 
Hui Lin
Ph.D. Candidate (http://hlin33.web.engr.illinois.edu/)
DEPEND (http://depend.csl.illinois.edu/)
ECE, Uni. of Illinois at Urbana-Champaign

_______________________________________________
Zeek mailing list
[email protected]
http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/zeek