Project

General

Profile

Actions

Bug #7235

closed

tls: a rule stops working since 7.0.5

Added by Ilya Bakhtin 4 months ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Difficulty:
Label:

Description

An alert from the rule that inspects unencrypted TLS negotiation is not reported since 7.0.5
The configuration is 'app-layer.protocols.tls.encryption-handling=bypass'

reproducer:
https://github.com/ilya-bakhtin/suricata-verify/tree/tls-bypass-missing-event


Subtasks 1 (0 open1 closed)

Bug #7290: tls: a rule stops working since 7.0.5 (7.0.x backport)ClosedVictor JulienActions
Actions #1

Updated by Ilya Bakhtin 4 months ago

I have already inspected the changes.

A condition is changed when StreamTcpDetectLogFlush is called. But at this moment FLOW_NOPAYLOAD_INSPECTION flag is already set to the flow flags by the application layer (TLS).
Thus StreamTcpPseudoPacketCreateDetectLogFlush prepares a pseudo packet with PKT_NOPAYLOAD_INSPECTION flag. See stream-tcp.c:7090

It looks like a pattern of communication is not commonly used. "Application Data" chunks exist in the same packet with "Server Hello". The change did not take care of this possibility.

- if (FlowChangeProto(p->flow)) {
+ // this is the first packet that sets no payload inspection
+ bool setting_nopayload =
+ p->flow->alparser &&
+ AppLayerParserStateIssetFlag(p->flow->alparser, APP_LAYER_PARSER_NO_INSPECTION) &&
+ !(p->flags & PKT_NOPAYLOAD_INSPECTION);
+ if (FlowChangeProto(p->flow) || setting_nopayload) {
StreamTcpDetectLogFlush(tv, fw->stream_thread, p->flow, p, &fw->pq);
+ if (setting_nopayload) {
+ FlowSetNoPayloadInspectionFlag(p->flow);
+ }

Actions #2

Updated by Ilya Bakhtin 4 months ago

A proposed fix, i'll prepare PR soon
The rationale - we have to inherit bypass options from the current packet instead of flow.
The flow object flags are already altered and must not be used at this stage.

--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@ -7083,10 +7083,10 @ static void StreamTcpPseudoPacketCreateDetectLogFlush(ThreadVars *tv,
np->vlan_idx = f->vlan_idx;
np->livedev = (struct LiveDevice_ *)f->livedev;

- if (f->flags & FLOW_NOPACKET_INSPECTION) {
+ if (parent->flags & PKT_NOPACKET_INSPECTION) {
DecodeSetNoPacketInspectionFlag(np);
}
- if (f->flags & FLOW_NOPAYLOAD_INSPECTION) {
+ if (parent->flags & PKT_NOPAYLOAD_INSPECTION) {
DecodeSetNoPayloadInspectionFlag(np);
}

Actions #4

Updated by Juliana Fajardini Reichow 3 months ago

  • Status changed from New to In Review
Actions #5

Updated by Victor Julien 3 months ago

  • Status changed from In Review to Resolved
  • Target version changed from TBD to 8.0.0-beta1
  • Label Needs backport to 7.0 added

Staged privately, will push after the weekend.

Actions #6

Updated by OISF Ticketbot 3 months ago

  • Subtask #7290 added
Actions #7

Updated by OISF Ticketbot 3 months ago

  • Label deleted (Needs backport to 7.0)
Actions

Also available in: Atom PDF