Commit Graph

12585 Commits (dab3274263865f7f2592d204141346911996d41b)
 

Author SHA1 Message Date
Jason Ish 54e62ddf71 http2: use generic tx iterator 3 years ago
Jason Ish 6cffecfe3e template: use generic tx iterator 3 years ago
Jason Ish a936755731 nfs: use generic tx iterator 3 years ago
Jason Ish 0188a01daf rfb: use generic tx iterator 3 years ago
Jason Ish b335409690 mqtt: use generic tx iterator 3 years ago
Jason Ish d71bcd82d9 modbus: use generic tx iterator 3 years ago
Jason Ish fcfc9876ce smb: use generic tx iterator 3 years ago
Jason Ish 049d43212e rust/app-layer: provide generic implementation of iterator
Create traits for app-layer State and Transaction that allow
a generic implementation of a transaction iterator that parser
can use when the follow the common pattern for iterating
transactions.

Also convert DNS to use the generic for testing purposes.
3 years ago
Eric Leblond 6d5f59696d profiling: fix profiling with sample rate
Rules profiling was returning invalid results when used with sample
rate. The problem was that the sample condition was run twice in the
packet flow. As a result, the second pass was not initializing the
variable storing the initial CPU ticks and the resulting performance
counters were reporting invalid values.

Bug: #4836.
3 years ago
Philippe Antoine 16f4e5f31c detect: file_data keyword works on nfs protocol
Ticket: #4839
3 years ago
Shivani Bhardwaj 26c7d3cc35 http2: remove needless borrows 3 years ago
Shivani Bhardwaj f3a1e3b92e core: Remove unneeded consts 3 years ago
Shivani Bhardwaj b5a123adb1 ssh: use Direction enum 3 years ago
Shivani Bhardwaj baf30cfc05 snmp: use Direction enum 3 years ago
Shivani Bhardwaj 89cb337930 smb: use Direction enum 3 years ago
Shivani Bhardwaj 8f9f78c2d0 sip: use Direction enum 3 years ago
Shivani Bhardwaj 11c438a07d nfs: use Direction enum 3 years ago
Shivani Bhardwaj a7ac79bed7 mqtt: use Direction enum 3 years ago
Shivani Bhardwaj 209e2f17fa krb: use Direction enum 3 years ago
Shivani Bhardwaj 243960a511 ike: use Direction enum 3 years ago
Shivani Bhardwaj ee5b300ccf http2: use Direction enum 3 years ago
Shivani Bhardwaj 0c6e9ac931 files: use Direction enum 3 years ago
Shivani Bhardwaj a19d2b4e1e dns: use Direction enum 3 years ago
Shivani Bhardwaj a866499bca dcerpc: use Direction enum 3 years ago
Shivani Bhardwaj 9512bfd729 core: add Direction enum
Ticket: 3832
3 years ago
Andreas Dolp b25350ee13 doc: Fix typo in documentation of rule keyword flow 3 years ago
Philippe Antoine 6cb6225b28 tcp: rejects FIN+SYN packets as invalid
Ticket: #4569

If a FIN+SYN packet is sent, the destination may keep the
connection alive instead of starting to close it.
In this case, a later SYN packet will be ignored by the
destination.

Previously, Suricata considered this a session reuse, and thus
used the sequence number of the last SYN packet, instead of
using the one of the live connection, leading to evasion.

This commit errors on FIN+SYN so that they do not get
processed as regular FIN packets.
3 years ago
Victor Julien 50e2b973ee stream/tcp: handle RST with MD5 or AO header
Special handling for RST packets if they have an TCP MD5 or AO header option.
The options hash can't be validated. The end host might be able to validate
it, as it can have a key/password that was communicated out of band.

The sender could use this to move the TCP state to 'CLOSED', leading to
a desync of the TCP session.

This patch builds on top of
843d0b7a10 ("stream: support RST getting lost/ignored")

It flags the receiver as having received an RST and moves the TCP state
into the CLOSED state. It then reverts this if the sender continues to
send traffic. In this case it sets the following event:

    stream-event:suspected_rst_inject;

Bug: #4710.
3 years ago
Philippe Antoine 3212fa7d2b ntp: fixes leak of de_state
Bug: #4752.
3 years ago
Philippe Antoine 28a3181a2d snmp: fixes leak of de_state
Bug: #4752.
3 years ago
Philippe Antoine f37240a3e2 smb: midstream probing checks for netbios message type
If it is available

Bug: #4620.
3 years ago
Philippe Antoine be617a3c1b protodetect: opposing side cannot change protocol
Ticket: #4562

As the data which triggered the opposing side
was the same protocol and not another one,
that means the protocol change failed.

Prevents a memory leak in later call of AppLayerParserParse
which would allocate a new state and leak the old one
3 years ago
Philippe Antoine f44bbbb9ad smtp: completes RSET transaction on last multiline
Bug: #4561.
3 years ago
Philippe Antoine f211a330dd swf: right input length for decompression
Also when compress_depth reaches buffer_len

Bug: #4536.
3 years ago
Philippe Antoine 8f8823b6f2 rust: right condition for both uint to be zero
Theay can overflow leading to their addition to be zero

If a NFS read reply indicates a count of 0xFFFFFFFF

Bug: #4680.
3 years ago
Philippe Antoine 689ac97d72 inspect: debug validation to ensure correct argument 3 years ago
Philippe Antoine c3339c853e detect: fixes InspectionBuffer id with transforms
When InspectionBufferGet gets called with base_id
Later InspectionBufferSetup must also be called with base_id

In case there were transforms, we had base_id != list_id

Not calling InspectionBufferSetup with the right id
resulted in leaving a dangling pointer,
because it was not added to det_ctx->inspect.to_clear_queue

Bug: #4681.
3 years ago
Victor Julien 244dd11c34 flow/manager: fix flows not evicted & freed in time
Flows have been shown to linger for a long time w/o giving up their
resources. This would lead to higher memory use and memcaps getting
reached.

Three main causes have been identified:

Slow passes hash passes. By default the flow manager will scan the
flow hash slowly. It is based on the flow timeout settings, and with
the default config it will take 4 minutes for a full scan to be
complete. This leaves a window for flows that are timed out to linger
for minutes longer than expected.

Flow Manager yields under pressure. The per row TryLock causes work
to be delayed more. The Flow manager will use trylock on a hash row
and will yield immediately if the row is busy. This means that it will
take a full pass before the row is revisited again. If the row holds
busy flows, this could happen many times in a row.

Flow Manager favors evicted flows over active flows. The Flow Manager
will only process the evicted flows if they are present. These flows
have been evicted by workers. The active flows on that hash row will
have to wait until the next hash pass. Of course by then there could
be more evicted flows.

Combined these factors could lead to flows not being considered for
freeing and logging for a very long time, potentially even indefinitly.

The patch addresses the latter two flow manager issues by no longer
using TryLock. It will now simply wait for the lock to be released and
then do its work on it. Additionally for each row both the evicted list
and the active flow list will be processed.

Bug: #4650.
3 years ago
Victor Julien ace349d4d9 af-packet: simplify tpacket-v2 setup code
Setup can no longer fail, so make the function void and remove dead
error checking code.
3 years ago
Victor Julien 2cbfcce0ac af-packet: PacketSetData can't fail; remove check
PacketSetData() can't fail unless the input pointer is NULL, which is
impossible from the af-packet paths calling it. Remove error check to
avoid possible branching.
3 years ago
Victor Julien 12252ba751 af-packet: fix if/down issues with tpacket-v2/autofp
The AFPSwitchState function would close the socket and free the
other resources when the interface went down _and_ the ref cnt was
0. However in autofp mode it was common to get to this point while
packets were still processed in the autofp worker threads, meaning
the ref cnt would not be 0. On the interface coming back up the
initialization code would overwrite the socket and rings, leading
to resource leaks.

Socket ref cnt is decremented from the v2 release callback. If the
callback would get to ref cnt 0, the packet would not be released
in the kernel, but it would (possibly) close the socket if the
iface was down, but not free other resources.

This patch changes the logic to first release the packet to the
kernel and then decrement the ref cnt and it makes the main receive
loop the only one responsible for opening and closing sockets. Wait
with closing the socket and rings until the ref count is 0, which can
happen after AFPSwitchState is called due to packets still being
processed by autofp worker threads.

Bug: #4803.
3 years ago
Victor Julien 3f8e15f70c af-packet: packet checks into debug validate check 3 years ago
Victor Julien 5e05fedc90 af-packet: hide all ebpf/bypass logic behind guards
Leave no runtime checks for bypass/ebpf/xdp if not compiled in.
3 years ago
Victor Julien e63db9d1d2 af-packet: minor code cleanups 3 years ago
Victor Julien e3d20acb98 af-packet: simplify socket handling in tpacket v3
Tpacket v3 only supports workers mode, which means the packet that would
reference a socket won't leave the thread. Therefore keeping a ref count
on the socket is not needed.

This patch removes the per packet reference count increment. The decrement
was missing, so this fixes the ref cnt handling so that after a iface up/
down capture can recover.

It should also lead to a minor performance increase as we avoid a round
of atomic operations per packet.

Bug: #4804.
Bug: #4801.
3 years ago
Victor Julien 6eaaafc360 af-packet: minor config parsing cleanup 3 years ago
Victor Julien be122ceb8f af-packet: remove obsolete define 3 years ago
Victor Julien 558930a192 af-packet: remove zero copy flag
Flag was always set for tpacket v2 and v3, which meant the check
for it in the packet setup paths were useless.
3 years ago
Victor Julien ad862fff37 af-packet: avoid flag colision with kernel
Avoid colision of TP_STATUS_USER_BUSY with TP_STATUS_TS_RAW_HARDWARE,
both were using bit 31.

Bug: #4800.
3 years ago
Victor Julien a022648b9e af-packet: fix soft lockup issues
The Suricata AF_PACKET code opens a socket per thread, then after some minor
setup enters a loop where the socket is poll()'d with a timeout. When the
poll() call returns a non zero positive value, the AF_PACKET ring will be
processed.

The ringbuffer processing logic has a pointer into the ring where we last
checked the ring. From this position we will inspect each frame until we
find a frame with tp_status == TP_STATUS_KERNEL (so essentially 0). This
means the frame is currently owned by the kernel.

There is a special case handling for starting the ring processing but
finding a TP_STATUS_KERNEL immediately. This logic then skip to the next
frame, rerun the check, etc until it either finds an initialized frame or
the last frame of the ringbuffer.

The problem was, however, that the initial uninitialized frame was possibly
(likely?) still being initialized by the kernel. A data race between the
notification through the socket (the poll()) and the updating of the
`tp_status` field in the frame could lead to a valid frame getting skipped.

Of note is that for example libpcap does not do frame scanning. Instead it
simply exits it ring processing loop. Also interesting is that libpcap uses
atomic loads and stores on the tp_status field.

This skipping of frames had 2 bad side effects:

1. in most cases, the buffer would be full enough that the frame would
   be processed in the next pass of the ring, but now the frame would
   out of order. This might have lead to packets belong to the same
   flow getting processed in the wrong order.

2. more severe is the soft lockup case. The skipped frame sits at ring
   buffer index 0. The rest of the ring has been cleared, after the
   initial frame was skipped. As our pass of the ring stops at the end
   of the ring (ptv->frame_offset + 1 == ptv->req.v2.tp_frame_nr) the code
   exits the ring processing loop at goes back to poll(). However, poll()
   will not indicate that there is more data, as the stale frame in the
   ring blocks the kernel from populating more frames beyond it. This
   is now a dead lock, as the kernel waits for Suricata and Suricata
   never touches the ring until it hears from the kernel.

   The scan logic will scan the whole ring at most once, so it won't
   reconsider the stale frame either.

This patch addresses the issues in several ways:

1. the startup "discard" logic was fixed to not skip over kernel
   frames. Doing so would get us in a bad state at start up.

2. Instead of scanning the ring, we now enter a busy wait loop
   when encountering a kernel frame where we didn't expect one. This
   means that if we got a > 0 poll() result, we'll busy wait until
   we get at least one frame.

3. Error handling is unified and cleaned up. Any frame error now
   returns the frame to the kernel and progresses the frame pointer.

4. If we find a frame that is owned by us (TP_STATUS_USER_BUSY) we
   yield to poll() immediately, as the next expected status of that
   frame is TP_STATUS_KERNEL.

5. the ring is no longer processed until the "end" of the ring (so
   highest index), but instead we process at most one full ring size
   per run.

6. Work with a copy of `tp_status` instead of accessing original touched
   also by the kernel.

Bug: #4785.
3 years ago