Commit Graph

28 Commits (999582b149ac664563fa2caa821055ba7c43699e)

Author SHA1 Message Date
Philippe Antoine 999582b149 app-layer: standardize includes with iwyu
Ticket: 945
4 weeks ago
Victor Julien a5effad936 frames: avoid possible undefined behavior
Code analyzer flagged FrameCopy as a possible source of UB due to
both pointers passed to memcpy being the same.

app-layer-frames.c: In function ‘FrameCopy’:
app-layer-frames.c:236:5: warning: overlapping buffers passed as arguments to ‘memcpy’ [-Wanalyzer-overlapping-buffers]
  236 |     memcpy(dst, src, sizeof(*dst));
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘FramePrune’: events 1-8
    │
    │  750 | static void FramePrune(Frames *frames, const TcpStream *stream, const bool eof)
    │      |             ^~~~~~~~~~
    │      |             |
    │      |             (1) entry to ‘FramePrune’
    │......
    │  766 |     for (uint16_t i = 0; i < frames->cnt; i++) {
    │      |                          ~~~~~~~~~~~~~~~
    │      |                            |
    │      |                            (2) following ‘true’ branch... ─>─┐
    │      |                                                              │
    │      |                                                              │
    │      |┌─────────────────────────────────────────────────────────────┘
    │  767 |│        if (i < FRAMES_STATIC_CNT) {
    │      |│           ~
    │      |│           |
    │      |└──────────>(3) ...to here
    │      |            (4) following ‘true’ branch (when ‘i <= 2’)... ─>─┐
    │      |                                                              │
    │      |                                                              │
    │      |┌─────────────────────────────────────────────────────────────┘
    │  768 |│            Frame *frame = &frames->sframes[i];
    │      |│                            ~~~~~~~~~~~~~~~~~~
    │      |│                                           |
    │      |└──────────────────────────────────────────>(5) ...to here
    │  769 |             FrameDebug("prune(s)", frames, frame);
    │  770 |             if (eof || FrameIsDone(frame, acked)) {
    │      |                ~
    │      |                |
    │      |                (6) following ‘false’ branch... ─>─┐
    │      |                                                   │
    │......
    │      |                                                   │
    │      |┌──────────────────────────────────────────────────┘
    │  779 |│                const uint64_t fle = FrameLeftEdge(stream, frame);
    │      |│                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │      |│                                     |
    │      |└────────────────────────────────────>(7) ...to here
    │      |                                      (8) calling ‘FrameLeftEdge’ from ‘FramePrune’
    │
    └──> ‘FrameLeftEdge’: event 9
           │
           │  257 | static inline uint64_t FrameLeftEdge(const TcpStream *stream, const Frame *frame)
           │      |                        ^~~~~~~~~~~~~
           │      |                        |
           │      |                        (9) entry to ‘FrameLeftEdge’
           │
         ‘FrameLeftEdge’: event 10
           │
           │suricata-common.h:323:27:
           │  323 |         #define BUG_ON(x) assert(!(x))
           │      |                           ^~~~~~
           │      |                           |
           │      |                           (10) following ‘false’ branch (when ‘frame_offset <= app_progress’)... ─>─┐
           │      |                                                                                                     │
util-validate.h:95:36: note: in expansion of macro ‘BUG_ON’
           │   95 | #define DEBUG_VALIDATE_BUG_ON(exp) BUG_ON((exp))
           │      |                                    ^~~~~~
app-layer-frames.c:266:5: note: in expansion of macro ‘DEBUG_VALIDATE_BUG_ON’
           │  266 |     DEBUG_VALIDATE_BUG_ON(frame_offset > app_progress);
           │      |     ^~~~~~~~~~~~~~~~~~~~~
           │
         ‘FrameLeftEdge’: event 11
           │
           │      |                                                                                                     │
           │      |┌────────────────────────────────────────────────────────────────────────────────────────────────────┘
           │  269 |│    if (frame->len < 0) {
           │      |│        ~~~~~^~~~~
           │      |│             |
           │      |└────────────>(11) ...to here
           │
    <──────┘
    │
  ‘FramePrune’: events 12-13
    │
    │  779 |                 const uint64_t fle = FrameLeftEdge(stream, frame);
    │      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │      |                                      |
    │      |                                      (12) returning to ‘FramePrune’ from ‘FrameLeftEdge’
    │......
    │  783 |                 FrameCopy(nframe, frame);
    │      |                 ~~~~~~~~~~~~~~~~~~~~~~~~
    │      |                 |
    │      |                 (13) calling ‘FrameCopy’ from ‘FramePrune’
    │
    └──> ‘FrameCopy’: events 14-15
           │
           │  234 | static void FrameCopy(Frame *dst, Frame *src)
           │      |             ^~~~~~~~~
           │      |             |
           │      |             (14) entry to ‘FrameCopy’
           │  235 | {
           │  236 |     memcpy(dst, src, sizeof(*dst));
           │      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           │      |     |
           │      |     (15) ⚠️  overlapping buffers passed as arguments to ‘memcpy’
           │
In file included from suricata-common.h:129,
                 from app-layer-frames.c:25:
/usr/include/string.h:47:14: note: the behavior of ‘memcpy’ is undefined for overlapping buffers
   47 | extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
      |              ^~~~~~
5 months ago
Philippe Antoine cb62d6d73d frames: do not free on log+flush packet
As we expect a second log+flush packet in the other direction

Ticket: 8336
5 months ago
Philippe Antoine 439f96dea7 rust: bindgen frames functions
Ticket: 7667
12 months ago
Philippe Antoine ad0f6e388e src: remove BUG_ON in packet path
Transforms them into debug validations

Ticket: 1484

Packet path is here considered FlowWorker and all lines
covered by SV tests are here fixed
1 year ago
Philippe Antoine ae1a4ef757 app-layer: make number of alprotos dynamic
Ticket: 5053

The names are now dynamically registered at runtime.
The AppProto alproto enum identifiers are still static for now.

This is the final step before app-layer plugins.
2 years ago
Philippe Antoine 911cd628fc frames: use dynamic number of app-layer protos
Ticket: 5053
2 years ago
Philippe Antoine 0d4efe0c0f app-layer: fix -Wshorten-64-to-32 warnings
Ticket: #6186

Warnings about downcast from 64 to 32 bits
2 years ago
Philippe Antoine e8438fdb58 app-layer: remove unused parameters 2 years ago
Victor Julien 2e5e3498a6 app-layer/frames: add by type getter
AppLayerFrameGetLastOpenByType: Returns the most recent frame with a type
with unknown length (-1).

Check if type is globally enabled first.
2 years ago
Victor Julien 803e8dd32e frames: add FrameGetLastOpenByType
Getter for the most recent frame with unknown length (-1).
2 years ago
Victor Julien c7402d2d01 frames: fix bounds check 2 years ago
Philippe Antoine a262e203f9 src: remove some unused parameters 2 years ago
Victor Julien 9287320330 license: fixup file headers
Thanks to Josh Soref.
4 years ago
Victor Julien c15d511064 frames: enable only used frames
Enable only frames that are actually used.

Ticket: #4979.
4 years ago
Victor Julien aa376a3b21 detect/frame: improve frame detection
Add a per frame progress tracker.
4 years ago
Victor Julien 6cbb5306c6 frame: add debug validation check 4 years ago
Victor Julien b43dc5a64a app-layer/frames: use absolute frame offset
Frame offset was already a 64 bit integer, so simplify things by
making it an absolute offset from the start of the stream.
4 years ago
Victor Julien 39d9b3adbe frames: implement generic <alproto>.stream frames
Add a hard coded <alproto>.stream option for all stream data for
a protocol.

Starts at stream offset 0 or at the point of a protocol upgrade
in case of STARTTLS or CONNECT.
4 years ago
Victor Julien 17cd41c887 frames: suppress cppcheck warning
src/app-layer-frames.c:471:5: warning: Identical condition 'stream_slice->input==NULL', second condition is always false [identicalConditionAfterEarlyExit]
    BUG_ON(stream_slice->input == NULL);
    ^
src/app-layer-frames.c:468:29: note: If condition 'stream_slice->input==NULL' is true, the function will return/exit
    if (stream_slice->input == NULL)
                            ^
src/app-layer-frames.c:471:5: note: Testing identical condition 'stream_slice->input==NULL'
    BUG_ON(stream_slice->input == NULL);
    ^
src/app-layer-frames.c:548:5: warning: Identical condition 'stream_slice->input==NULL', second condition is always false [identicalConditionAfterEarlyExit]
    BUG_ON(stream_slice->input == NULL);
    ^
src/app-layer-frames.c:545:29: note: If condition 'stream_slice->input==NULL' is true, the function will return/exit
    if (stream_slice->input == NULL)
                            ^
src/app-layer-frames.c:548:5: note: Testing identical condition 'stream_slice->input==NULL'
    BUG_ON(stream_slice->input == NULL);
    ^
4 years ago
Victor Julien 39cf5b151a src: includes cleanup
Work towards making `suricata-common.h` only introduce system headers
and other things that are independent of complex internal Suricata
data structures.

Update files to compile after this.

Remove special DPDK handling for strlcpy and strlcat, as this caused
many compilation failures w/o including DPDK headers for all files.

Remove packet macros from decode.h and move them into their own file,
turn them into functions and rename them to match our function naming
policy.
4 years ago
Victor Julien e250ef6402 debug: remove empty header 4 years ago
Philippe Antoine 02f2602dde src: rework includes as per cppclean 4 years ago
Victor Julien 3a7d09edfc detect/frame: get data using stream callback
Inspect only data that has already been consumed by the
app-layer parser. This allows for simpler progress tracking.
5 years ago
Philippe Antoine f30975fb16 app-layer: fix integer warnings
Ticket: 4516
5 years ago
Victor Julien 013fb2dde3 frames: remove dead condition in eof check 5 years ago
Victor Julien c96d22e8a1 frames: support UDP frames
UDP frames point to the UDP packet payloads.

The frames are removed after each packet.

Ticket: #4983.
5 years ago
Victor Julien 1556e86c7d app/frames: initial support
The idea of stream frames is that the applayer parsers can tag PDUs and
other arbitrary frames in the stream while parsing. These frames can then
be inspected from the rule language. This will allow rules that are more
precise and less costly.

The frames are stored per direction in the `AppLayerParserState` and will only
be initialized when actual frames are in use. The per direction storage has a
fixed size static portion and dynamic support for a larger number. This is done
for effeciency.

When the Stream Buffer slides, frames are updated as they use offsets relative
to the stream. A negative offset is used for frames that started before the
current window.

Frames have events to inspect/log parser errors that don't fit the TX model.

Frame id starts at 1. So implementations can keep track of frame ids where 0
is not set.

Frames affect TCP window sliding. The frames keep a "left edge" which
signifies how much data to keep for frames that are still in progress.
5 years ago