From 33e3fdabe444e8ccfd86ad05da816c566a53f4e2 Mon Sep 17 00:00:00 2001 From: blotus Date: Thu, 21 Dec 2023 11:51:04 +0100 Subject: [PATCH] Appsec additional fixes (#2676) --- cmd/crowdsec-cli/hubappsec.go | 4 ++ cmd/crowdsec-cli/item_metrics.go | 60 ++++++++++++++++++- cmd/crowdsec-cli/utils_table.go | 13 ++++ .../modules/appsec/appsec_runner.go | 35 ++++++----- pkg/acquisition/modules/appsec/metrics.go | 12 ++-- pkg/appsec/appsec.go | 1 - pkg/appsec/appsec_rule/modsecurity.go | 1 + pkg/appsec/request.go | 6 +- pkg/appsec/request_test.go | 4 +- pkg/hubtest/hubtest_item.go | 2 +- 10 files changed, 108 insertions(+), 30 deletions(-) diff --git a/cmd/crowdsec-cli/hubappsec.go b/cmd/crowdsec-cli/hubappsec.go index 364519dd4..8371f49b3 100644 --- a/cmd/crowdsec-cli/hubappsec.go +++ b/cmd/crowdsec-cli/hubappsec.go @@ -48,6 +48,10 @@ cscli appsec-configs list crowdsecurity/vpatch`, func NewCLIAppsecRule() *cliItem { inspectDetail := func(item *cwhub.Item) error { + //Only show the converted rules in human mode + if csConfig.Cscli.Output != "human" { + return nil + } appsecRule := appsec.AppsecCollectionConfig{} yamlContent, err := os.ReadFile(item.State.LocalPath) diff --git a/cmd/crowdsec-cli/item_metrics.go b/cmd/crowdsec-cli/item_metrics.go index b99992613..e6f27ae5d 100644 --- a/cmd/crowdsec-cli/item_metrics.go +++ b/cmd/crowdsec-cli/item_metrics.go @@ -33,7 +33,8 @@ func ShowMetrics(hubItem *cwhub.Item) error { } } case cwhub.APPSEC_RULES: - log.Error("FIXME: not implemented yet") + metrics := GetAppsecRuleMetric(csConfig.Cscli.PrometheusUrl, hubItem.Name) + appsecMetricsTable(color.Output, hubItem.Name, metrics) default: // no metrics for this item type } return nil @@ -176,6 +177,63 @@ func GetScenarioMetric(url string, itemName string) map[string]int { return stats } +func GetAppsecRuleMetric(url string, itemName string) map[string]int { + stats := make(map[string]int) + + stats["inband_hits"] = 0 + stats["outband_hits"] = 0 + + results := GetPrometheusMetric(url) + for idx, fam := range results { + if !strings.HasPrefix(fam.Name, "cs_") { + continue + } + log.Tracef("round %d", idx) + for _, m := range fam.Metrics { + metric, ok := m.(prom2json.Metric) + if !ok { + log.Debugf("failed to convert metric to prom2json.Metric") + continue + } + name, ok := metric.Labels["rule_name"] + if !ok { + log.Debugf("no rule_name in Metric %v", metric.Labels) + } + if name != itemName { + continue + } + + band, ok := metric.Labels["type"] + if !ok { + log.Debugf("no type in Metric %v", metric.Labels) + } + + value := m.(prom2json.Metric).Value + fval, err := strconv.ParseFloat(value, 32) + if err != nil { + log.Errorf("Unexpected int value %s : %s", value, err) + continue + } + ival := int(fval) + + switch fam.Name { + case "cs_appsec_rule_hits": + switch band { + case "inband": + stats["inband_hits"] += ival + case "outband": + stats["outband_hits"] += ival + default: + continue + } + default: + continue + } + } + } + return stats +} + func GetPrometheusMetric(url string) []*prom2json.Family { mfChan := make(chan *dto.MetricFamily, 1024) diff --git a/cmd/crowdsec-cli/utils_table.go b/cmd/crowdsec-cli/utils_table.go index f3179e40f..b1e4b6950 100644 --- a/cmd/crowdsec-cli/utils_table.go +++ b/cmd/crowdsec-cli/utils_table.go @@ -25,6 +25,19 @@ func listHubItemTable(out io.Writer, title string, items []*cwhub.Item) { t.Render() } +func appsecMetricsTable(out io.Writer, itemName string, metrics map[string]int) { + t := newTable(out) + t.SetHeaders("Inband Hits", "Outband Hits") + + t.AddRow( + strconv.Itoa(metrics["inband_hits"]), + strconv.Itoa(metrics["outband_hits"]), + ) + + renderTableTitle(out, fmt.Sprintf("\n - (AppSec Rule) %s:", itemName)) + t.Render() +} + func scenarioMetricsTable(out io.Writer, itemName string, metrics map[string]int) { if metrics["instantiation"] == 0 { return diff --git a/pkg/acquisition/modules/appsec/appsec_runner.go b/pkg/acquisition/modules/appsec/appsec_runner.go index 3d6d27cf3..1436ea942 100644 --- a/pkg/acquisition/modules/appsec/appsec_runner.go +++ b/pkg/acquisition/modules/appsec/appsec_runner.go @@ -318,7 +318,7 @@ func (r *AppsecRunner) handleRequest(request *appsec.ParsedRequest) { // time spent to process in band rules inBandParsingElapsed := time.Since(startInBandParsing) - AppsecInbandParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized}).Observe(inBandParsingElapsed.Seconds()) + AppsecInbandParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized, "appsec_engine": request.AppsecEngine}).Observe(inBandParsingElapsed.Seconds()) if request.Tx.IsInterrupted() { r.handleInBandInterrupt(request) @@ -334,26 +334,29 @@ func (r *AppsecRunner) handleRequest(request *appsec.ParsedRequest) { r.AppsecRuntime.Response.SendAlert = false r.AppsecRuntime.Response.SendEvent = true - //to measure the time spent in the Application Security Engine for OutOfBand rules - startOutOfBandParsing := time.Now() + //FIXME: This is a bit of a hack to avoid confusion with the transaction if we do not have any inband rules. + //We should probably have different transaction (or even different request object) for inband and out of band rules + if len(r.AppsecRuntime.OutOfBandRules) > 0 { + //to measure the time spent in the Application Security Engine for OutOfBand rules + startOutOfBandParsing := time.Now() - err = r.ProcessOutOfBandRules(request) - if err != nil { - logger.Errorf("unable to process OutOfBand rules: %s", err) - return + err = r.ProcessOutOfBandRules(request) + if err != nil { + logger.Errorf("unable to process OutOfBand rules: %s", err) + return + } + + // time spent to process out of band rules + outOfBandParsingElapsed := time.Since(startOutOfBandParsing) + AppsecOutbandParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized}).Observe(outOfBandParsingElapsed.Seconds()) + if request.Tx.IsInterrupted() { + r.handleOutBandInterrupt(request) + } } - - // time spent to process out of band rules - outOfBandParsingElapsed := time.Since(startOutOfBandParsing) - AppsecOutbandParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized}).Observe(outOfBandParsingElapsed.Seconds()) - // time spent to process inband AND out of band rules globalParsingElapsed := time.Since(startGlobalParsing) - AppsecGlobalParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized}).Observe(globalParsingElapsed.Seconds()) + AppsecGlobalParsingHistogram.With(prometheus.Labels{"source": request.RemoteAddrNormalized, "appsec_engine": request.AppsecEngine}).Observe(globalParsingElapsed.Seconds()) - if request.Tx.IsInterrupted() { - r.handleOutBandInterrupt(request) - } } func (r *AppsecRunner) Run(t *tomb.Tomb) error { diff --git a/pkg/acquisition/modules/appsec/metrics.go b/pkg/acquisition/modules/appsec/metrics.go index 8efa56313..132759338 100644 --- a/pkg/acquisition/modules/appsec/metrics.go +++ b/pkg/acquisition/modules/appsec/metrics.go @@ -6,27 +6,27 @@ var AppsecGlobalParsingHistogram = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Help: "Time spent processing a request by the Application Security Engine.", Name: "cs_appsec_parsing_time_seconds", - Buckets: []float64{0.005, 0.01, 0.025, 0.050, 0.1, 0.2, 0.3, 0.4, 0.5, 1}, + Buckets: []float64{0.0001, 0.00025, 0.0005, 0.001, 0.0025, 0.0050, 0.01, 0.025, 0.05, 0.1, 0.25}, }, - []string{"source"}, + []string{"source", "appsec_engine"}, ) var AppsecInbandParsingHistogram = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Help: "Time spent processing a request by the inband Application Security Engine.", Name: "cs_appsec_inband_parsing_time_seconds", - Buckets: []float64{0.005, 0.01, 0.025, 0.050, 0.1, 0.2, 0.3, 0.4, 0.5, 1}, + Buckets: []float64{0.0001, 0.00025, 0.0005, 0.001, 0.0025, 0.0050, 0.01, 0.025, 0.05, 0.1, 0.25}, }, - []string{"source"}, + []string{"source", "appsec_engine"}, ) var AppsecOutbandParsingHistogram = prometheus.NewHistogramVec( prometheus.HistogramOpts{ Help: "Time spent processing a request by the Application Security Engine.", Name: "cs_appsec_outband_parsing_time_seconds", - Buckets: []float64{0.005, 0.01, 0.025, 0.050, 0.1, 0.2, 0.3, 0.4, 0.5, 1}, + Buckets: []float64{0.0001, 0.00025, 0.0005, 0.001, 0.0025, 0.0050, 0.01, 0.025, 0.05, 0.1, 0.25}, }, - []string{"source"}, + []string{"source", "appsec_engine"}, ) var AppsecReqCounter = prometheus.NewCounterVec( diff --git a/pkg/appsec/appsec.go b/pkg/appsec/appsec.go index 4550990b2..011b371f7 100644 --- a/pkg/appsec/appsec.go +++ b/pkg/appsec/appsec.go @@ -30,7 +30,6 @@ const ( hookOnMatch ) -// @tko : todo - debug mode func (h *Hook) Build(hookStage int) error { ctx := map[string]interface{}{} diff --git a/pkg/appsec/appsec_rule/modsecurity.go b/pkg/appsec/appsec_rule/modsecurity.go index 1698953df..7b98206c6 100644 --- a/pkg/appsec/appsec_rule/modsecurity.go +++ b/pkg/appsec/appsec_rule/modsecurity.go @@ -21,6 +21,7 @@ var zonesMap map[string]string = map[string]string{ "PROTOCOL": "REQUEST_PROTOCOL", "URI": "REQUEST_URI", "RAW_BODY": "REQUEST_BODY", + "FILENAMES": "FILES", } var transformMap map[string]string = map[string]string{ diff --git a/pkg/appsec/request.go b/pkg/appsec/request.go index 4ae05c854..f9ff4c169 100644 --- a/pkg/appsec/request.go +++ b/pkg/appsec/request.go @@ -78,7 +78,7 @@ func (r *ReqDumpFilter) WithEmptyHeadersFilters() *ReqDumpFilter { return r } -func (r *ReqDumpFilter) WithHeadersContentFilters(filter string) *ReqDumpFilter { +func (r *ReqDumpFilter) WithHeadersContentFilter(filter string) *ReqDumpFilter { r.HeadersContentFilters = append(r.HeadersContentFilters, filter) return r } @@ -114,7 +114,7 @@ func (r *ReqDumpFilter) WithEmptyArgsFilters() *ReqDumpFilter { return r } -func (r *ReqDumpFilter) WithArgsContentFilters(filter string) *ReqDumpFilter { +func (r *ReqDumpFilter) WithArgsContentFilter(filter string) *ReqDumpFilter { r.ArgsContentFilters = append(r.ArgsContentFilters, filter) return r } @@ -247,7 +247,7 @@ func (r *ReqDumpFilter) GetFilteredRequest() *ParsedRequest { } func (r *ReqDumpFilter) ToJSON() error { - fd, err := os.CreateTemp("/tmp/", "crowdsec_req_dump_*.json") + fd, err := os.CreateTemp("", "crowdsec_req_dump_*.json") if err != nil { return fmt.Errorf("while creating temp file: %w", err) } diff --git a/pkg/appsec/request_test.go b/pkg/appsec/request_test.go index b05ecbde6..f8333e4e5 100644 --- a/pkg/appsec/request_test.go +++ b/pkg/appsec/request_test.go @@ -63,7 +63,7 @@ func TestBodyDumper(t *testing.T) { Headers: map[string][]string{"test1": {"toto"}}, }, filter: func(r *ReqDumpFilter) *ReqDumpFilter { - return r.WithHeadersContentFilters("tata") + return r.WithHeadersContentFilter("tata") }, }, { @@ -153,7 +153,7 @@ func TestBodyDumper(t *testing.T) { Args: map[string][]string{"test": {"lol"}}, }, filter: func(r *ReqDumpFilter) *ReqDumpFilter { - return r.WithArgsContentFilters("toto") + return r.WithArgsContentFilter("toto") }, }, } diff --git a/pkg/hubtest/hubtest_item.go b/pkg/hubtest/hubtest_item.go index 6fce22df8..3bab4b412 100644 --- a/pkg/hubtest/hubtest_item.go +++ b/pkg/hubtest/hubtest_item.go @@ -89,7 +89,7 @@ const ( TestBouncerApiKey = "this_is_a_bad_password" - DefaultNucleiTarget = "http://127.0.0.1:80/" + DefaultNucleiTarget = "http://127.0.0.1:7822/" DefaultAppsecHost = "127.0.0.1:4241" )