Changing semantics of ConnSizeAnalyzer

[seth]

I think we should change what the conn size analyzer is measuring. It currently measures the size of the connection from the IP header down (or up, depending on how you look at it). From my perspective that data is rarely (if ever?) useful. What is more useful is a counted value for the connection size. c$(orig|resp)$size takes it's measurement from sequence counting and can get confused in some cases (chinese firewall sending RST packets for instance).

This is the patch I'm proposing:

diff --git a/scripts/base/init-bare.bro b/scripts/base/init-bare.bro
index 859a69f..21a9b60 100644
--- a/scripts/base/init-bare.bro
+++ b/scripts/base/init-bare.bro
@@ -66,10 +66,10 @@ type endpoint: record {
 
        ## Number of packets on the wire
        ## Set if :bro:id:`use_conn_size_analyzer` is true.
-       num_pkts: count &optional;      
-       ## Number of IP-level bytes on the wire
+       counted_pkts: count &optional;
+       ## Number of content bytes on the wire
        ## Set if :bro:id:`use_conn_size_analyzer` is true.
-       num_bytes_ip: count &optional;  
+       counted_bytes: count &optional;
 };
 
 type endpoint_stats: record {
diff --git a/src/ConnSizeAnalyzer.cc b/src/ConnSizeAnalyzer.cc
index a1b892f..5d0efcd 100644
--- a/src/ConnSizeAnalyzer.cc
+++ b/src/ConnSizeAnalyzer.cc
@@ -39,12 +39,12 @@ void ConnSize_Analyzer::DeliverPacket(int len, const u_char* data, bool is_orig,
 
        if ( is_orig )
                {
-               orig_bytes += ip->TotalLen();
+               orig_bytes += len;
                orig_pkts ++;
                }
        else
                {
-               resp_bytes += ip->TotalLen();
+               resp_bytes += len;
                resp_pkts ++;
                }
        }

If no one has a problem with this, I'd like to make the change for the 2.0 release because I'm having trouble currently with counting bytes for the SSH analyzer and we're getting more false positives than we should be seeing.

[gregor]

Sounds good.
However, as a measurement guy I like the IP level bytes. How about counting both payload and IP level bytes? (It's fine if we only report the payload bytes per default, but I'd really like to keep support for IP level counting in the core)

-Gregor

[robin]

I'm reluctant to count only payload bytes as I find that not very
intuitive and also non-standard (NetFlow? for example counts IP bytes
as well). It feels like we'd be tuning a general mechanism to a
specific case (SSH login detection). The sequence number calcuation
seems the right thing to use here, and I'd prefer to fix that instead.

That said, for now I can see doing both as Gregor suggested, however I
would log the IP bytes only and use payload bytes in scripts where
helpful.

Robin

[seth]

I'm going to remove this ticket from a milestone for now and we can revisit it later. I'll file another ticket to remind us to find and fix the sequence counting bugs for the 2.1 release.

[gregor]

On 12/18/11 15:57 , Bro Tracker wrote:

#724: Changing semantics of ConnSizeAnalyzer?

---

Reporter: seth | Owner:

Type: Problem | Status: new

Priority: High | Milestone: Bro2.0

Component: Bro | Version:

Resolution: | Keywords:

---

Comment (by robin):

I'm reluctant to count only payload bytes as I find that not very
intuitive and also non-standard (NetFlow? for example counts IP bytes
as well).

Actually IIRC, NetFlow? counts IP-payload (i.e., including tcp/udp headers but not IP headers).