Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tcp midstream fin 5437 v2 #8445

Closed

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • relax json schema for alert metadata
  • pick up midstream session even with TCP FIN flag set

suricata-verify-pr: 1051

Replaces #8320 with adding a check so that this only happens for FIN + data packets

As this is a free field and can have any key based on a rule
@catenacyber
Copy link
Contributor Author

cf OISF/suricata-verify#1051

"properties": {
"affected_product": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"attack_target": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"created_at": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"deployment": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"former_category": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"malware_family": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"policy": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"signature_severity": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"tag": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
},
"updated_at": {
"type": "array",
"minItems": 1,
"items": {
"type": "string"
}
}
},
"additionalProperties": false
"additionalProperties": true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense as this is free-form data from the rule language. Could be anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For our SV it might make sense to be stricter, so we can spot issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think it is an issue to use something which is not in this list, as the ET rules do in OISF/suricata-verify#1051

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For our SV it might make sense to be stricter, so we can spot issues?

I think this would require a separate schema for S-V. The idea of the schema is to produce something re-usable for documentation and code generation purposes, so it doesn't make sense to limit this to only the data that is seen in Suricata-Verify when it really is free-form data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still include the commonly used fields like we have currently, but do allow additional fields.

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW2_single_stats_chk
.tcp.ssn_from_cache 264554 339652 128.39%
.tcp.ssn_from_pool 628437 553339 88.05%
SURI_TLPW2_autofp_stats_chk
.tcp.ssn_from_cache 298547 375943 125.92%
.tcp.ssn_from_pool 594444 517048 86.98%
SURI_TLPW1_stats_chk
.tcp.ssn_from_cache 63596 78950 124.14%
.tcp.ssn_from_pool 139220 123866 88.97%
.tcp.rst 128116 113462 88.56%
.app_layer.error.dns_udp.parser 1276 0 -
.ftp.memuse 862 348 40.37%
SURI_TLPR1_stats_chk
.flow.memuse 580947328 537162128 92.46%
.tcp.ssn_from_cache 3644947 6050401 165.99%
.tcp.ssn_from_pool 8484810 6079323 71.65%
.tcp.synack 7538139 10048598 133.3%
.tcp.rst 5264608 6666912 126.64%
.app_layer.error.ftp-data.gap 0 1 -
.app_layer.error.dns_udp.parser 28003 0 -
.http.memuse 16440 2104 12.8%
.ftp.memuse 13827 11093 80.23%
IPS_AFP_stats_chk
.tcp.segment_from_pool 17089689 15752213 92.17%
TREX_GENERIC_stats_chk
.tcp.segment_from_pool 15728353 14684559 93.36%

Pipeline 12094

This was referenced Feb 10, 2023
@victorjulien
Copy link
Member

I've merged in #8536 this without the schema update, as I want to discuss that some more. Should block this work though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants