snort-devel February 2014 archive
Main Archive Page > Month Archives  > snort-devel archives
snort-devel: Re: [Snort-devel] Patch for Stream5 TCP direction

Re: [Snort-devel] Patch for Stream5 TCP direction

From: John Eure <john.eure_at_nospam>
Date: Fri Feb 21 2014 - 03:10:42 GMT
To: "Carter Waxman (cwaxman)" <cwaxman@cisco.com>

The issues are:

1) Stream5IsPafActiveTcp() returns incorrect results
2) Stream5ActivatePafTcp() fails when it should work

What I found is that Stream5IsPafActiveTcp() returned the result for the
direction opposite to the direction specified in its arguments, and that
Stream5ActivatePafTcp() activated PAF in the direction opposite to the
direction specified in its arguments. Most of the time, this is hidden,
because Stream5ActivatePafTcp() is called twice in a row, for both
directions, on a stream that has PAF inactive in both directions. But when
called on a stream with PAF active in one direction but not in the other,
the results are incorrect.

This is all, of course, assuming that "PKT_FROM_CLIENT",
"SSN_DIR_FROM_CLIENT", "to_server == true", and "c2s == true" should all
refer to the same direction, that is, from the client to the server. As
far as I can tell, they do: they all refer to the same direction
everywhere, except for in 2 of the 3 Stream5 API functions that have an
argument named "to_server". So this patch changes those 2 functions.

(It would, of course, also be possible to leave the inside of those
functions the same, and just change the argument names from "to_server" to
"to_client" or "from_server", but that would add inconsistency to the API.)

Does that answer your question?

John

On Thu, Feb 20, 2014 at 6:48 AM, Carter Waxman (cwaxman)
<cwaxman@cisco.com>wrote:

> Hi John,
>
> What is the issue you are trying to fix? Is there a particular behavior
> this is causing?
>
> -Carter
>
>
> From: John Eure <john.eure@gmail.com>
> Date: Wednesday, February 19, 2014 6:32 PM
> To: snort-devel <snort-devel@lists.sourceforge.net>
> Subject: [Snort-devel] Patch for Stream5 TCP direction
>
> Hello, snort-devel,
>
> I've got a patch that fixes a tiny issue with some of the Stream5 TCP
> API functions.
>
> Stream5IsPafActiveTcp() takes a boolean called "to_server", but which
> actually behaves as if it were "to_client"; I fixed that by swapping the
> references to the TcpSession->client/server fields inside the function.
> Stream5ActivatePafTcp() has a similar problem, but there are two variables,
> one for the StreamTracker and one for the FlushMgr. I switched the
> FlushMgr references, and I know that's correct, but I left the
> StreamTracker references alone, and I *think* that's correct, but I'm not
> sure. I don't use this function in my code anymore, so it's hard for me to
> verify that everything works correctly, but I know at least the FlushMgr
> should switch, so I did that.
>
> There's also a related function, Stream5GetPAFUserDataTcp(), which was
> already working correctly, so I left it alone. I've included a few
> comments that have helped me keep track of which direction is which. And
> I've updated the http_inspect preprocessor, which was the only one to call
> stream_api->is_paf_active(), so that it continues to do the right thing.
>
> On a related subject, I very much appreciate the recent renaming from
> SSN_DIR_CLIENT to SSN_DIR_FROM_CLIENT - it helped me find and fix a few
> issues with my own code. At that point I went through and changed all of
> my relevant names to have either "s2c" or "c2s" in them (borrowing that
> terminology from the PAF module), and the result is that my code is now
> simpler, less buggy, and less prone to cause insanity from trying to figure
> out what "client" and "server" actually mean when applied to streams. I
> heartily recommend the practice. :-)
>
> Thanks for everything,
> John Eure
>

------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk

_______________________________________________
Snort-devel mailing list
Snort-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel

Please visit http://blog.snort.org for the latest news about Snort!