Suricata

What rules are you using ?

You should still get DNS metadata if you use dns keywords (and same for TLS)

This is the simplified version of my rules:

reject ip $HOME_NET any -> any any (msg:"Blocked - Has Blocked"; flow: to_server; flowbits:isset,HAS.BLOCKED.ALERT; noalert; classtype:block; sid:1; rev:1; gid:1111;)
reject tls $HOME_NET any -> any any (msg:"Blocked - TLS"; flow:to_server; flowbits:isnotset,HAS.ALLOWED.ALERT.TLS; flowbits:isnotset,HAS.BLOCKED.ALERT; luajit:tls.lua; flowbits:set,HAS.BLOCKED.ALERT; classtype:block-tls; sid:5; rev:2; gid:1111;)
reject dns $HOME_NET any -> any any (msg:"Blocked - DNS"; flow:to_server; luajit:dns.lua; flowbits:set,HAS.BLOCKED.ALERT; flowbits:set,HAS.BLOCKED.ALERT.DNS; classtype:block-dns; sid:2; rev:3; gid:131415;)
alert ip $HOME_NET any -> any any (msg:"Allowed Connection Attempt"; flow:to_server,not_established; flowbits:isset,CONNECTION_ATTEMPT; flowbits:isnotset,HAS.BLOCKED.ALERT; flowbits:set,HAS.ALLOWED.ALERT; flowbits:unset,CONNECTION_ATTEMPT; classtype:allow; sid:4; rev:5;)
alert dns $HOME_NET any -> any any (msg:"Access Control Allowed - DNS"; flow:to_server; flowbits:isnotset,HAS.BLOCKED.ALERT; flowbits:isnotset,HAS.BLOCKED.ALERT.DNS; dns.query; bsize:>0; flowbits:set,HAS.ALLOWED.ALERT; classtype:allow; sid:6; rev:4;)
alert tls $HOME_NET any -> any any (msg:"Allowed - TLS"; flow:to_server;  flowbits:isnotset,HAS.BLOCKED.ALERT; flowbits:isnotset,HAS.ALLOWED.ALERT.TLS; flowbits:set,HAS.ALLOWED.ALERT.TLS; classtype:allow; sid:7; rev:2;)

Tls:¶

In the test of tls, the lua would return allow for both the ip and tls.
I added below print above the AlertQueueAppend in DetectRulePacketRules :

SCLogInfo("AlertQueueAppend with %lu, flags 0x%x, alstate %p", txid, alert_flags, pflow ? pflow->alstate : NULL);

The tls packets tested were a simple google.com session handshake.

• For the connection/ip alert:
  

  [30] 22/11/2024 -- 19:58:41 - (detect.c:831) <Info> (DetectRulePacketRules) -- AlertQueueAppend with 18446744073709551615, flags 0x0, alstate (nil)
  

• For the tls:
  

  [30] 22/11/2024 -- 19:58:41 - (detect.c:831) <Info> (DetectRulePacketRules) -- AlertQueueAppend with 18446744073709551615, flags 0x0, alstate 0x7668e8405e40
  

Dns¶

With the same log above + a similar log in DetectRunTx before AlertQueueAppend, and the dns lua would block.

The dns udp packet:

0000   45 00 00 37 4a d6 40 00 40 11 c3 c9 0a 0a 0c 02
0010   0a 0a 0c 01 89 95 00 35 00 23 18 82 ec 36 01 00
0020   00 01 00 00 00 00 00 00 05 63 68 65 61 74 03 63
0030   6f 6d 00 00 01 00 01

The logs look like this:

• For the connection/ip alert:
  

  [30] 22/11/2024 -- 20:10:15 - (detect.c:831) <Info> (DetectRulePacketRules) -- AlertQueueAppend with 18446744073709551615, flags 0x0, alstate 0x7d662c401e90
  

• For the dns alert:
  

  [30] 22/11/2024 -- 20:10:15 - (detect.c:1606) <Info> (DetectRunTx) -- Tx AlertQueueAppend with 0, flags 0xa
  

So for the ip/connection alert, the PACKET_ALERT_FLAG_TX is not set, and thus no dns app data. I mentioned this might be the new expected behaviour as the ip rule does not contain the dns keyword, but it is still a backward incompatible change that would break many eve log dependent services. I don't see why we couldn't log the application data in this case, as it is a single packet.

Could you test https://github.com/OISF/suricata/pull/12153 ?

I have tested the patch, the patch crashes when pflow is NULL. Since the condition would never get the flag set when pflow->alstate is NULL, I moved the pflow and pflow->alstate check outside, this is more readable than before and avoids the crashes.
This fix now works for both of my cases.

        if (pflow && pflow->alstate) {
            if ((alert_flags & PACKET_ALERT_FLAG_STREAM_MATCH) ||
                (s->alproto != ALPROTO_UNKNOWN && pflow->proto == IPPROTO_UDP) ||
                (AppLayerParserGetTxCnt(pflow, pflow->alstate) == 1))
            {
                // if there is a stream match (TCP), or
                // a UDP specific app-layer signature,
                // or only one transaction
                // try to use the good tx for the packet direction
                uint8_t dir =
                        (p->flowflags & FLOW_PKT_TOCLIENT) ? STREAM_TOCLIENT : STREAM_TOSERVER;
                txid = AppLayerParserGetTransactionInspectId(pflow->alparser, dir);
                alert_flags |= PACKET_ALERT_FLAG_TX;
            }
        }

Indeed, I saw that as well and did https://github.com/OISF/suricata/pull/12158

Yeah, I found it on github and made a comment there too. I just think the if layout can be re-organized a bit to be more readable.

thanks a lot for the prompt fix! https://github.com/OISF/suricata/pull/12161 LGTM.