exception/policy: add stats counters - v3.1

[jufajardini]

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/5816

Previous PR: #8735

Describe changes:

• include exception policy stats counters for app-layer errors in summary form, too (see output example)
• allow configuring whether or not exception policy counters are logged if 0
• allow configuring whether or not exception policy error counters are logged for each app-proto
• remove stats counters logging when exception policy == ignore
• remove stats counters for exceptions that are never valid, for certain scenarios (based on https://docs.suricata.io/en/latest/configuration/exception-policies.html)

TODO:

• document exception policy stats counters
• check what is the overflow caused when enabling delta counters or thread logs
• confirm if we are not trying to log counters for cases when exception policies are not valid (such as specific situations in IDS or IPS).
• better align stats.log - as the new counters size character is longer than what we currently have in the stats
• update the json.schema to include delta counters.

Output examples:
tcp

  "tcp": {
    "syn": 1,
    "synack": 1,
    "rst": 3,
    "active_sessions": 0,
    "sessions": 1,
    "ssn_memcap_drop": 0,
    "ssn_from_cache": 0,
    "ssn_from_pool": 1,
    "ssn_memcap_exception_policy": {
      "reject": 0,
      "bypass": 0,
      "pass_flow": 0,
      "pass_packet": 0,
      "drop_flow": 0,
      "drop_packet": 0
    },
    "pseudo": 0,
    "pseudo_failed": 0,
    "invalid_checksum": 0,
    "midstream_pickups": 0,
    "midstream_exception_policy": {
      "reject": 0,
      "bypass": 0,
      "pass_flow": 0,
      "drop_flow": 0
    },

app-layer -> summary, logging zero values, logging per-app-proto:

"app_layer": {
    "error": {
      "exception_policy": {
        "reject": 0,
        "bypass": 0,
        "pass_flow": 0,
        "pass_packet": 1,
        "drop_flow": 0,
        "drop_packet": 0
      },
      "http": {
        "gap": 0,
        "alloc": 0,
        "parser": 0,
        "internal": 0,
        "exception_policy": {
          "reject": 0,
          "bypass": 0,
          "pass_flow": 0,
          "pass_packet": 0,
          "drop_flow": 0,
          "drop_packet": 0
        }
      },

app-layer -> not logging per app-proto, nor zeroes:

  "app_layer": {
    "error": {
      "exception_policy": {
        "drop_flow": 1
      },
      "tls": {
        "gap": 0,
        "alloc": 0,
        "parser": 1,
        "internal": 0
      },

app-layer-> logging per app-proto, no zeroes:

  "tls": {
    "gap": 0,
    "alloc": 0,
    "parser": 1,
    "internal": 0,
    "exception_policy": {
      "pass_packet": 1
    }
  },
  "smb": {
    "gap": 0,
    "alloc": 0,
    "parser": 0,
    "internal": 0,
    "exception_policy": {}
  },

suricata-verify-pr: OISF/suricata-verify#1617

[codecov]

Codecov Report

Attention: 213 lines in your changes are missing coverage. Please review.

Comparison is base (c3b3c11) 82.28% compared to head (a4ee217) 82.21%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10264      +/-   ##
==========================================
- Coverage   82.28%   82.21%   -0.07%     
==========================================
  Files         977      977              
  Lines      271950   272276     +326     
==========================================
+ Hits       223784   223865      +81     
- Misses      48166    48411     +245     

Flag	Coverage Δ
fuzzcorpus	63.31% <27.16%> (-0.09%)	⬇️
suricata-verify	61.43% <34.87%> (-0.09%)	⬇️
unittests	62.76% <17.02%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[suricata-qa]

Information: QA ran without warnings.

Pipeline 17791

[victorjulien]

scan-build:

app-layer.c: In function 'AppLayerIncrErrorExcPolicyCounter':
app-layer.c:213:9: warning: 'g_id' may be used uninitialized [-Wmaybe-uninitialized]
  213 |         StatsIncr(tv, g_id);
      |         ^~~~~~~~~~~~~~~~~~~
app-layer.c:176:14: note: 'g_id' was declared here
  176 |     uint16_t g_id;
      |              ^~~~
app-layer.c:212:27: warning: The left operand of '>' is a garbage value [core.UndefinedBinaryOperatorResult]
    if (likely(tv && g_id > 0)) {
                     ~~~~ ^
./util-optimize.h:32:42: note: expanded from macro 'likely'
#define likely(expr) __builtin_expect(!!(expr), 1)
                                         ^~~~
1 warning generated.

[jufajardini]

Scan Build complaint is about this bit.

[jufajardini]

This doens't look like the best way to do this. (This and what follows). Moreover, with the introduction of summary counters for App Layer error exception policy counters, maybe this isn't the best option? Not sure yet...

[jufajardini]

scan-build:

app-layer.c: In function 'AppLayerIncrErrorExcPolicyCounter':
app-layer.c:213:9: warning: 'g_id' may be used uninitialized [-Wmaybe-uninitialized]
  213 |         StatsIncr(tv, g_id);
      |         ^~~~~~~~~~~~~~~~~~~
app-layer.c:176:14: note: 'g_id' was declared here
  176 |     uint16_t g_id;
      |              ^~~~
app-layer.c:212:27: warning: The left operand of '>' is a garbage value [core.UndefinedBinaryOperatorResult]
    if (likely(tv && g_id > 0)) {
                     ~~~~ ^
./util-optimize.h:32:42: note: expanded from macro 'likely'
#define likely(expr) __builtin_expect(!!(expr), 1)
                                         ^~~~
1 warning generated.

Thanks! This isn't what's causing the other issue, though.

[jufajardini]

Probably not the best solution, but must get this config before app-layer counters setup.

[jufajardini]

This is part of what causes the address sanitizer error. Removing the wrong portion of the stats, most likely.

[jufajardini]

Followed by: #10364