Yes, you are right. <br><br>I have changed it now to use the flow keyword from the rule.<br><br><div class="gmail_quote">On Mon, Oct 15, 2012 at 11:23 AM, Victor Julien <span dir="ltr"><<a href="mailto:victor@inliniac.net" target="_blank">victor@inliniac.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On 10/14/2012 01:25 AM, I. Sanchez wrote:<br>
> It is fixed now. It was a silly issue with one "if" (plus a few other<br>
> minor issues in the option string parser).<br>
><br>
> Now everything seems to be working ok.<br>
><br>
> The match function looks like this now:<br>
><br>
> static int DetectGeoipMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx,<br>
><br>
> Packet *p, Signature *s, SigMatch *m)<br>
><br>
> {<br>
> DetectGeoipData *geoipdata = (DetectGeoipData *)m->ctx;<br>
><br>
> int match = 0;<br>
> int matches = 0;<br>
><br>
><br>
> if (PKT_IS_IPV4(p))<br>
><br>
> {<br>
> if (geoipdata->flags & GEOIP_MATCH_SRC_FLAG || geoipdata->flags<br>
> & GEOIP_MATCH_BOTH_FLAG)<br>
<br>
</div>You could write this as<br>
if (geoipdata->flags & (GEOIP_MATCH_SRC_FLAG|GEOIP_MATCH_BOTH_FLAG)<br>
<div class="im"><br>
><br>
> {<br>
> /* if there is a flow get SRC IP of the flow, not packet */<br>
> if (p->flowflags & FLOW_PKT_TOCLIENT)<br>
<br>
</div>Not sure I understand why the flow direction is checked here? The<br>
keyword should inspect the pkt src I think, regardless of flow.<br>
<br>
If a user wants only a certain flow direction checked, the flow keyword<br>
can be used:<br>
<br>
flow:to_client; geoip:src,CN;<br>
<br>
Cheers,<br>
Victor<br>
<div><div class="h5"><br>
><br>
> /* the dst (from server to client) is our src */<br>
> match = CheckGeoMatchIPv4(geoipdata,<br>
> GET_IPV4_DST_ADDR_U32(p));<br>
><br>
> else<br>
> match = CheckGeoMatchIPv4(geoipdata,<br>
> GET_IPV4_SRC_ADDR_U32(p));<br>
><br>
> if (match)<br>
> {<br>
> if (geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
> matches++;<br>
> else<br>
><br>
> return 1;<br>
> }<br>
> }<br>
> if (geoipdata->flags & GEOIP_MATCH_DST_FLAG || geoipdata->flags<br>
> & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
> {<br>
> /* if there is a flow get DST IP of the flow, not packet */<br>
> if (p->flowflags & FLOW_PKT_TOCLIENT)<br>
><br>
> /* the src (from server to client) is our dst */<br>
> match = CheckGeoMatchIPv4(geoipdata,<br>
> GET_IPV4_SRC_ADDR_U32(p));<br>
><br>
> else<br>
> match = CheckGeoMatchIPv4(geoipdata,<br>
> GET_IPV4_DST_ADDR_U32(p));<br>
><br>
> if (match)<br>
> {<br>
> if (geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
> matches++;<br>
> else<br>
><br>
> return 1;<br>
> }<br>
> }<br>
> /* if matches == 2 is because match-on is "both" */<br>
> if (matches == 2)<br>
><br>
> return 1;<br>
> }<br>
><br>
><br>
> return 0;<br>
> }<br>
><br>
><br>
><br>
> On Sat, Oct 13, 2012 at 9:46 PM, I. Sanchez <<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a><br>
</div></div><div class="im">> <mailto:<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a>>> wrote:<br>
><br>
> Ok, I have done an initial implementation (just country geolocation<br>
> for now). It is available at<br>
> <a href="https://github.com/owlsec/suricata/tree/geoip" target="_blank">https://github.com/owlsec/suricata/tree/geoip</a><br>
><br>
> When checking a packet, I take into account the flow source and<br>
> destination IPs for the match-on condition, if a flow exists.<br>
> However in my tests I have seen it is not working well... a<br>
> geoip:src,US; rule will be triggered as well when talking HTTP to<br>
</div>> <a href="http://google.com" target="_blank">google.com</a> <<a href="http://google.com" target="_blank">http://google.com</a>> from a non US source IP address.<br>
<div><div class="h5">><br>
> I am not sure about the reason of this behavior, so perhaps somebody<br>
> could let me know what is wrong here.<br>
><br>
> <a href="https://github.com/owlsec/suricata/blob/geoip/src/detect-geoip.c" target="_blank">https://github.com/owlsec/suricata/blob/geoip/src/detect-geoip.c</a><br>
><br>
> The relevant function is this one:<br>
><br>
> static int DetectGeoipMatch(ThreadVars *t, DetectEngineThreadCtx<br>
> *det_ctx,<br>
><br>
><br>
> Packet *p, Signature *s, SigMatch *m)<br>
><br>
><br>
> {<br>
> DetectGeoipData *geoipdata = (DetectGeoipData *)m->ctx;<br>
><br>
><br>
> int match = 0;<br>
> int matches = 0;<br>
><br>
> uint32_t ip;<br>
><br>
> if (PKT_IS_IPV4(p))<br>
><br>
> {<br>
> if (geoipdata->flags & GEOIP_MATCH_SRC_FLAG ||<br>
> geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
><br>
> {<br>
> /* if there is a flow get SRC IP of the flow, not packet */<br>
> if (p->flowflags & FLOW_PKT_TOCLIENT)<br>
><br>
> ip = GET_IPV4_DST_ADDR_U32(p); /* the dst (from<br>
> server to client) is our src */<br>
><br>
> else<br>
> ip = GET_IPV4_SRC_ADDR_U32(p);<br>
><br>
> match = CheckGeoMatchIPv4(geoipdata, ip);<br>
><br>
> if (match && geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
><br>
> matches++;<br>
> else<br>
><br>
> return 1;<br>
> }<br>
> if (geoipdata->flags & GEOIP_MATCH_DST_FLAG ||<br>
> geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
><br>
> {<br>
> /* if there is a flow get DST IP of the flow, not packet */<br>
> if (p->flowflags & FLOW_PKT_TOCLIENT)<br>
><br>
> ip = GET_IPV4_SRC_ADDR_U32(p); /* the src (from<br>
> server to client) is our dst */<br>
><br>
> else<br>
> ip = GET_IPV4_DST_ADDR_U32(p);<br>
><br>
> match = CheckGeoMatchIPv4(geoipdata, ip);<br>
><br>
> if (match && geoipdata->flags & GEOIP_MATCH_BOTH_FLAG)<br>
><br>
><br>
> matches++;<br>
> else<br>
><br>
> return 1;<br>
> }<br>
><br>
> /* if matches == 2 is because match-on is "both" */<br>
> if (matches == 2)<br>
><br>
> return 1;<br>
> }<br>
><br>
><br>
> return 0;<br>
> }<br>
><br>
><br>
><br>
> On Fri, Oct 12, 2012 at 11:35 AM, I. Sanchez<br>
</div></div><div class="im">> <<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a> <mailto:<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a>>> wrote:<br>
><br>
> Yes, I forgot to mention it. Negation will be supported.<br>
><br>
><br>
> On Fri, Oct 12, 2012 at 10:03 AM, Peter Manev<br>
</div><div class="im">> <<a href="mailto:petermanev@gmail.com">petermanev@gmail.com</a> <mailto:<a href="mailto:petermanev@gmail.com">petermanev@gmail.com</a>>> wrote:<br>
><br>
> Excellent - thank you.<br>
> comments bellow ...<br>
><br>
> On Thu, Oct 11, 2012 at 10:07 PM, I. Sanchez<br>
> <<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a><br>
</div><div class="im">> <mailto:<a href="mailto:sanchezmartin.ji@gmail.com">sanchezmartin.ji@gmail.com</a>>> wrote:<br>
><br>
> Good idea, I will implement multiple<br>
> conditions(countries) in the same rule. Let's use the<br>
> <match-on><condition>+ syntax where match-on can be src,<br>
> dst, both or any.<br>
><br>
><br>
> alert http any any -> any any (msg:"GEOIP: IP located in<br>
</div>> US/Germany/Canada/France";*geoip:src,US,DE,CA,FR*;<br>
<div class="im">> sid:3450002; rev:1;)<br>
><br>
> I can also support geoip:US; by assuming geoip:any,US; ,<br>
> for simplicity.<br>
><br>
><br>
> I agree with the assumption here - i think it is good to<br>
> assume so.<br>
> I was thinking further on the matter and I am not sure if i<br>
> am starting to sound annoying - but wouldn't it be nice if<br>
> we can also negate geoip? :<br>
> alert http any any -> any any (msg:"GEOIP: IP destination<br>
</div>> *NOT* located in US/Canada";**geoip:*dst,!*US,CA;<br>
<div class="im">> sid:3450002; rev:1;)<br>
><br>
><br>
><br>
> Regarding the city support, indeed the MaxMind DBs in<br>
> their free versions support cities in addition to<br>
> countries although the accuracy drops from 99.5% (for<br>
> countries) to 78% in US (for cities), and I guess much<br>
> less accuracy in other countries.<br>
><br>
> In the commercial DBs, they apparently support regions,<br>
> organizations...<br>
> <a href="http://www.maxmind.com/en/geolocation_landing" target="_blank">http://www.maxmind.com/en/geolocation_landing</a><br>
><br>
> For now I will just implement support for countries, but<br>
> we should take this into account for the keyword syntax.<br>
> I see some options:<br>
><br>
</div>> * Autodetect city vs country. I could detect whether<br>
<div class="im">> the condition is a known country code, and assume<br>
> city otherwise. However this will not work for<br>
> regions, organizations...<br>
</div>> * Allow -for future versions- the check type as an<br>
<div class="im">> optional param of the <match-on> condition. ie:<br>
> geoip:src,city,Madrid;<br>
><br>
><br>
> this would be awesome in my opinion.<br>
><br>
> Regards,<br>
><br>
><br>
><br>
><br>
><br>
> On Thu, Oct 11, 2012 at 9:02 PM, Peter Manev<br>
</div><div class="im">> <<a href="mailto:petermanev@gmail.com">petermanev@gmail.com</a> <mailto:<a href="mailto:petermanev@gmail.com">petermanev@gmail.com</a>>> wrote:<br>
><br>
> Hi,<br>
><br>
> I think i love that new geoip keyword - thank you<br>
> for the efforts !<br>
><br>
> A couple of suggestions/requests if I may:<br>
><br>
> 1.I agree/like the proposal - but I wonder if it<br>
> would be possible to include multiples(maybe up to a<br>
> certain number [32 or something] ) of countries - like:<br>
> alert http any any -> any any (msg:"GEOIP: IP<br>
> located in<br>
</div>> US/Germany/Canada/France";*geoip:src,US,DE,CA,FR*;<br>
<div class="im">> sid:3450002; rev:1;)<br>
><br>
</div>> 2. As there is - *src, dst, both* - i think it would<br>
> be nice if there is also "*any*" -<br>
<div class="im">> alert http any any -> any any (msg:"GEOIP: some<br>
</div>> traffic to/from the Cayman Islands";*geoip:any,KY*;<br>
<div class="im">> sid:3450005; rev:1;)<br>
> any - meaning either source or destination.<br>
><br>
> thanks a bunch!<br>
><br>
><br>
> On Thu, Oct 11, 2012 at 6:42 PM, Victor Julien<br>
</div>> <<a href="mailto:victor@inliniac.net">victor@inliniac.net</a> <mailto:<a href="mailto:victor@inliniac.net">victor@inliniac.net</a>>><br>
<div><div class="h5">> wrote:<br>
><br>
> On 10/11/2012 06:16 PM, I. Sanchez wrote:<br>
> > Hi,<br>
> ><br>
> > I am implementing support for IP address<br>
> country geolocation in<br>
> > Suricata, and I wanted to ask your opinion<br>
> about the syntax to be used<br>
> > for the geoip keyword options.<br>
> ><br>
> ><br>
> <a href="https://redmine.openinfosecfoundation.org/issues/559" target="_blank">https://redmine.openinfosecfoundation.org/issues/559</a><br>
> ><br>
> > The keyword options would be:<br>
> ><br>
> > * Country code. ie: US<br>
> > * Match condition: match on source IP, match<br>
> on destination IP, or<br>
> > match on both.<br>
> ><br>
> > What do you think would be the best syntax for<br>
> this?<br>
> ><br>
> > Some possibilities:<br>
> ><br>
> > * geoip:<src|dst|both>,<countrycode>;<br>
> > o alert http any any -> any any<br>
> (msg:"GEOIP: IP located in<br>
> > US";*geoip:src,US*;sid:3450002;rev:1;)<br>
> > * geoip:<countrycode>,<src|dst|both>;<br>
> > o alert http any any -> any any<br>
> (msg:"GEOIP: IP located in<br>
> > US";*geoip:US,src*;sid:3450002;rev:1;)<br>
><br>
> Thanks for picking this up!<br>
><br>
> Doesn't the geoip also allow for other types of<br>
> data, such as city? I'm<br>
> sure that if we have this in Suricata ppl will<br>
> be interested in buying<br>
> the more detailed databases as well.<br>
><br>
> --<br>
> ---------------------------------------------<br>
> Victor Julien<br>
> <a href="http://www.inliniac.net/" target="_blank">http://www.inliniac.net/</a><br>
> PGP: <a href="http://www.inliniac.net/victorjulien.asc" target="_blank">http://www.inliniac.net/victorjulien.asc</a><br>
> ---------------------------------------------<br>
><br>
> _______________________________________________<br>
> Oisf-devel mailing list<br>
> <a href="mailto:Oisf-devel@openinfosecfoundation.org">Oisf-devel@openinfosecfoundation.org</a><br>
</div></div>> <mailto:<a href="mailto:Oisf-devel@openinfosecfoundation.org">Oisf-devel@openinfosecfoundation.org</a>><br>
<div class="im">> <a href="https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel" target="_blank">https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel</a><br>
><br>
><br>
><br>
><br>
> --<br>
> Regards,<br>
> Peter Manev<br>
><br>
><br>
> _______________________________________________<br>
> Oisf-devel mailing list<br>
> <a href="mailto:Oisf-devel@openinfosecfoundation.org">Oisf-devel@openinfosecfoundation.org</a><br>
</div>> <mailto:<a href="mailto:Oisf-devel@openinfosecfoundation.org">Oisf-devel@openinfosecfoundation.org</a>><br>
<div class="HOEnZb"><div class="h5">> <a href="https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel" target="_blank">https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel</a><br>
><br>
><br>
><br>
><br>
><br>
> --<br>
> Regards,<br>
> Peter Manev<br>
><br>
><br>
><br>
><br>
<br>
<br>
--<br>
---------------------------------------------<br>
Victor Julien<br>
<a href="http://www.inliniac.net/" target="_blank">http://www.inliniac.net/</a><br>
PGP: <a href="http://www.inliniac.net/victorjulien.asc" target="_blank">http://www.inliniac.net/victorjulien.asc</a><br>
---------------------------------------------<br>
<br>
_______________________________________________<br>
Oisf-devel mailing list<br>
<a href="mailto:Oisf-devel@openinfosecfoundation.org">Oisf-devel@openinfosecfoundation.org</a><br>
<a href="https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel" target="_blank">https://lists.openinfosecfoundation.org/mailman/listinfo/oisf-devel</a><br>
</div></div></blockquote></div><br>