From: Russ Combs Date: Fri, 22 Aug 2014 17:49:40 +0000 (-0400) Subject: fp FIXITs X-Git-Tag: 3.0.0-233~1419^2~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=59805debb827fab7dd6292fa2f08836bab9517e6;p=thirdparty%2Fsnort3.git fp FIXITs --- diff --git a/ChangeLog b/ChangeLog index 0d94f0c39..bfecaa839 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,7 @@ fixed) -- fixed action leak -- testing visibility=hidden -- more FIXITs +-- fp related FIXITs 115 -- remove share.h diff --git a/src/detection/fpcreate.cc b/src/detection/fpcreate.cc index 12057aa06..d6f5394ec 100644 --- a/src/detection/fpcreate.cc +++ b/src/detection/fpcreate.cc @@ -946,11 +946,12 @@ static PmType get_pm_type(CursorActionType cat) return PM_TYPE__MAX; } -static PatternMatchData * get_fp_content(OptTreeNode *otn) +void set_fp_content(OptTreeNode *otn) { OptFpList *ofl; CursorActionType curr_cat = CAT_SET_RAW; FpFoo best; + PatternMatchData* pmd = nullptr; for (ofl = otn->opt_func; ofl != NULL; ofl = ofl->next) { @@ -970,8 +971,17 @@ static PatternMatchData * get_fp_content(OptTreeNode *otn) tmp->pm_type = get_pm_type(curr_cat); - if (tmp->fp) - return tmp; + if ( tmp->fp ) + { + if ( pmd ) + ParseError("only one fast_pattern content per rule allowed"); + + else if ( !pmd_can_be_fp(tmp, curr_cat) ) + ParseError("content ineligible for fast_pattern matcher"); + + else + pmd = tmp; + } if ( !pmd_can_be_fp(tmp, curr_cat) ) continue; @@ -981,7 +991,29 @@ static PatternMatchData * get_fp_content(OptTreeNode *otn) if ( curr.is_better(best) ) best = curr; } - return best.pmd; + if ( !pmd && best.pmd ) + best.pmd->fp = 1; +} + +static PatternMatchData* get_fp_content(OptTreeNode *otn) +{ + OptFpList *ofl; + + for (ofl = otn->opt_func; ofl != NULL; ofl = ofl->next) + { + if ( !ofl->context ) + continue; + + if ( ofl->type != RULE_OPTION_TYPE_CONTENT ) + continue; + + PatternMatchData* pmd = get_pmd(ofl); + assert(pmd); + + if ( pmd->fp ) + return pmd; + } + return nullptr; } static int fpFinishPortGroupRule( @@ -1148,8 +1180,8 @@ static int fpAddPortGroupRule( if ( !pg || !otn ) return -1; - // skip builtin rules - if ( !otn->sigInfo.text_rule ) // FIXIT-H must be set for so rules too! + // skip builtin rules, continue for text and so rules + if ( !otn->sigInfo.text_rule ) return -1; /* Rule not enabled */ diff --git a/src/detection/fpcreate.h b/src/detection/fpcreate.h index fe1c312ee..4b6daf5f9 100644 --- a/src/detection/fpcreate.h +++ b/src/detection/fpcreate.h @@ -200,10 +200,12 @@ PORT_GROUP * fpGetServicePortGroupByOrdinal(sopg_table_t *, int, int, int16_t); ** Shows the event stats for the created FastPacketDetection */ void fpShowEventStats(SnortConfig*); -typedef int (*OtnWalkFcn)(int, struct RuleTreeNode *, struct OptTreeNode *); +typedef int (*OtnWalkFcn)(int, struct RuleTreeNode *, struct OptTreeNode*); void fpWalkOtns(int, OtnWalkFcn); void fpDynamicDataFree(void *); +void set_fp_content(struct OptTreeNode*); + const char * PatternRawToContent(const char *pattern, int pattern_len); #endif /* __FPCREATE_H__ */ diff --git a/src/flow/flow.cc b/src/flow/flow.cc index abefe75c0..df9c671bf 100644 --- a/src/flow/flow.cc +++ b/src/flow/flow.cc @@ -34,13 +34,6 @@ unsigned FlowData:: flow_id = 0; -// FIXIT-H can't inline SO_PUBLIC ctor and dtor in header or we get problems: -// ld: warning: direct access in FlowData::FlowData(unsigned int, -// Inspector*) to global weak symbol vtable for FlowData means the weak -// symbol cannot be overridden at runtime. This was likely caused by -// different translation units being compiled with different visibility -// settings. - SO_PUBLIC FlowData::FlowData(unsigned u, Inspector* ph) { assert(u > 0); diff --git a/src/ips_options/ips_content.cc b/src/ips_options/ips_content.cc index 4fbc66774..62e79fa15 100644 --- a/src/ips_options/ips_content.cc +++ b/src/ips_options/ips_content.cc @@ -227,28 +227,6 @@ static PatternMatchData* new_pmd() return pmd; } -// FIXIT-H must ensure that fast_pattern is applied to -// a fast_pattern inspection buffer -static int fast_pattern_count(OptTreeNode *otn, int list_type) -{ - OptFpList* fpl = otn ? otn->opt_func : nullptr; - int c = 0; - - while ( fpl ) - { - if ( fpl->type == list_type ) - { - ContentOption* opt = (ContentOption*)fpl->context; - PatternMatchData* pmd = opt->get_data(); - - if ( pmd->fp ) - c++; - } - fpl = fpl->next; - } - return c; -} - static int32_t parse_int( const char* data, const char* tag, int low = -65535, int high = 65535) { @@ -278,17 +256,11 @@ static int32_t parse_int( return value; } -static void validate_content( - PatternMatchData* pmd, OptTreeNode* otn) +static void finalize_content(PatternMatchData* pmd, OptTreeNode*) { - if ( fast_pattern_count(otn, RULE_OPTION_TYPE_CONTENT) > 1 ) - { - ParseError("only one content per rule may be used for fast pattern matching."); - return; - } - if ( pmd->negated ) - pmd->last_check = (PmdLastCheck*)SnortAlloc(get_instance_max() * sizeof(*pmd->last_check)); + pmd->last_check = (PmdLastCheck*)SnortAlloc( + get_instance_max() * sizeof(*pmd->last_check)); } static void make_precomp(PatternMatchData * idx) @@ -821,17 +793,17 @@ bool ContentModule::set(const char*, Value& v, SnortConfig*) pmd->no_case = 1; else if ( v.is("fast_pattern") ) - pmd->fp = 1; // FIXIT-H must ensure current buffer is fp compatible + pmd->fp = 1; else if ( v.is("fast_pattern_offset") ) { pmd->fp_offset = v.get_long(); - pmd->fp = 1; // FIXIT-H must ensure current buffer is fp compatible + pmd->fp = 1; } else if ( v.is("fast_pattern_length") ) { pmd->fp_length = v.get_long(); - pmd->fp = 1; // FIXIT-H must ensure current buffer is fp compatible + pmd->fp = 1; } else return false; @@ -857,7 +829,7 @@ static IpsOption* content_ctor(Module* p, OptTreeNode * otn) { ContentModule* m = (ContentModule*)p; PatternMatchData* pmd = m->get_data(); - validate_content(pmd, otn); + finalize_content(pmd, otn); return new ContentOption(pmd); } diff --git a/src/main/modules.cc b/src/main/modules.cc index 126cb94ac..fc57e3921 100644 --- a/src/main/modules.cc +++ b/src/main/modules.cc @@ -546,8 +546,9 @@ bool ReferencesModule::set(const char*, Value& v, SnortConfig*) static const Parameter alerts_params[] = { { "alert_file", Parameter::PT_STRING, nullptr, nullptr, - "set the alert output file name (FIXIT-H delete if not used)" }, + "set the alert output file name" }, // FIXIT-H delete if not used + // FIXIT-L move to fast, full, syslog and delete from here { "alert_with_interface_name", Parameter::PT_BOOL, nullptr, "false", "include interface in alert info (fast, full, or syslog only)" }, diff --git a/src/main/snort.cc b/src/main/snort.cc index ab72ec611..3172101a1 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -820,7 +820,7 @@ DAQ_Verdict ProcessPacket( p->proto_bits = PROTO_BIT__OTHER; #if 0 - // FIXIT-H required until decoders are fixed (josh) + // FIXIT-J required until decoders are fixed else if ( !p->family && (p->proto_bits & PROTO_BIT__IP) ) p->proto_bits &= ~PROTO_BIT__IP; #endif diff --git a/src/managers/ips_manager.cc b/src/managers/ips_manager.cc index 07ac61b16..e0f3ab9aa 100644 --- a/src/managers/ips_manager.cc +++ b/src/managers/ips_manager.cc @@ -222,7 +222,7 @@ bool IpsManager::option_set( } if ( !set_arg(current_module, current_params, opt, val, sc) ) - ParseError("invalid argument %s:%s = %s\n", key, opt, val); + ParseError("invalid argument %s:%s = %s", key, opt, val); if ( current_params->is_positional() ) ++current_params; @@ -251,7 +251,7 @@ bool IpsManager::option_end( if ( mod && !mod->end(key, 0, sc) ) { - ParseError("can't finalize %s\n", key); + ParseError("can't finalize %s", key); current_keyword = nullptr; return false; } diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index 1f0b968d8..888de7982 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -1481,8 +1481,10 @@ OptTreeNode* parse_rule_open(SnortConfig* sc, RuleTreeNode& rtn, bool stub) OptTreeNode* otn = (OptTreeNode *)SnortAlloc(sizeof(OptTreeNode)); otn->state = (OtnState*)SnortAlloc(sizeof(OtnState)*get_instance_max()); + if ( !stub ) + otn->sigInfo.generator = GENERATOR_SNORT_ENGINE; + otn->chain_node_number = otn_count; - otn->sigInfo.generator = GENERATOR_SNORT_ENGINE; otn->proto = rtn.proto; otn->enabled = ScDefaultRuleState(); @@ -1516,13 +1518,15 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot ParseError("SO rule %s not loaded.", otn->soid); else { - // FIXIT-L why isn't this set already? (don't hardcode) + // FIXIT-L gid may be overwritten here otn->sigInfo.generator = GENERATOR_SNORT_SHARED; entered = true; return so_opts; } } + set_fp_content(otn); + /* The IPs in the test node get free'd in ProcessHeadNode if there is * already a matching RTN. The portobjects will get free'd when the * port var table is free'd */ @@ -1549,7 +1553,8 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot // FIXIT-L need more reliable way of knowing type of rule instead of hard // coding these gids - if ( otn->sigInfo.generator == 1 ) + // do GIDs actually matter anymore (w/o conflict with builtins)? + if ( otn->sigInfo.generator == GENERATOR_SNORT_ENGINE ) { otn->sigInfo.text_rule = true; detect_rule_count++; @@ -1561,6 +1566,9 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot } else { + if ( !otn->sigInfo.generator ) + ParseError("gid must set in builtin rules"); + if ( otn->num_detection_opts ) ParseError("builtin rules do not support detection options");