Decouple stream bypass from TLS encrypted bypass v6 #12082
Conversation
Decouple app.protocols.tls.encryption-handling and stream.bypass. There's no apparent reason why encrypted TLS bypass traffic should depend on stream bypass, as these are unrelated features. Ticket: 6788
lukashino
requested review from
catenacyber,
jufajardini,
victorjulien and
a team
as code owners
|
NOTE: This PR may contain new authors. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12082 +/- ##
==========================================
- Coverage 83.37% 83.37% -0.01%
==========================================
Files 910 910
Lines 257556 257579 +23
==========================================
+ Hits 214748 214762 +14
- Misses 42808 42817 +9
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: ERROR: QA failed on SURI_TLPR1_alerts_cmp.
Pipeline 23244 |
|
^ This should get a rerun, kinda weird that it didn't bypass dcerpc where that should not be affected by the PR at all - my assumption is that dcerpc_tcp runs unencrypted. |
| @@ -549,6 +561,16 @@ pub extern "C" fn rs_ssh_hassh_is_enabled() -> bool { | |||
| hassh_is_enabled() | |||
| } | |||
|
|
|||
| #[no_mangle] | |||
| pub extern "C" fn rs_ssh_enable_bypass() { | |||
There was a problem hiding this comment.
old style "style". New is "SCSshEnableBypass", so our C style with SC prefix. This applies only to the global functions.
|
|
||
| fn hassh_is_enabled() -> bool { | ||
| HASSH_ENABLED.load(Ordering::Relaxed) | ||
| } | ||
|
|
||
| fn enc_bypass_is_enabled() -> bool { |
There was a problem hiding this comment.
style: here and beyond, lets replace enc with encrypt or encryption for readability.
| bool enc_bypass = SSH_CONFIG_DEFAULT_BYPASS; | ||
| ConfNode *enc_handle = ConfGetNode("app-layer.protocols.ssh.encryption-handling"); | ||
| if (enc_handle != NULL && enc_handle->val != NULL) { | ||
| if (strcmp(enc_handle->val, "full") == 0) { |
There was a problem hiding this comment.
"full" to me means APP_LAYER_PARSER_NO_INSPECTION isn't set either. It just means full processing and inspection of the rest of the flow even if encrypted phase has started
| # What to do when the encrypted communications start: | ||
| # - bypass: stop processing this flow as much as possible. | ||
| # Offload flow bypass to kernel or hardware if possible. | ||
| # - full: keep tracking and inspection as normal |
There was a problem hiding this comment.
not what the code does, see above
|
Information: QA ran without warnings. Pipeline 23247 |
Following up on #11886
Redmine ticket: https://redmine.openinfosecfoundation.org/issues/6788
Describe changes:
v6
v5
v4
v3
encryption-handlingallowing to choose whether to continue inspection on SSH once it turns encryptedSV_BRANCH=OISF/suricata-verify#2114