Commit Graph

74 Commits (c17df004ed609cd19e12c87b48cfce60bb016951)

Author SHA1 Message Date
Victor Julien 86a363b1bc decode/tcp: improve pointer hygene
Avoid NULL pointer calculations.
1 year ago
Victor Julien 68804b8c4b decode/tcp: move tcph into L4 packet data
To reduce Packet size.

Ticket: #6938.
1 year ago
Victor Julien cca912e9b3 decode/tcp: add and use PacketIsTCP inline func
To prepare future changes to the Packet header pointers.

Ticket: #5517.
1 year ago
Victor Julien 2001ddc583 decode: use macro's instead of direct ptr checks
To prepare future changes to the Packet header pointers.

Ticket: #5517.
1 year ago
Victor Julien 1777e0314e decode/tcp: remove unused macro
SET_OPTS is now unused, so remove.
1 year ago
Victor Julien 6b8093d7b8 decode/tcp: reduce size needed for SACK tracking
No longer use a pointer, but rather an offset.

Part of effort to make Packet more compact.

Ticket: #6938.
1 year ago
Victor Julien 7be0678c3a decode/tcp: reduce size needed for tracking WSCALE
Part of effort to make Packet more compact.

Ticket: #6938.
1 year ago
Victor Julien 6cab2480e5 decode/tcp: reduce space needed for tracking TFO
Part of effort to make Packet more compact.

Ticket: #6938.
1 year ago
Victor Julien 6a23fafa5f decode/tcp: reduce space needed for MSS tracking
Part of effort to make Packet more compact.

Ticket: #6938.
1 year ago
Victor Julien 9632c2f570 decode/tcp: optimize SACKOK storage
Take less space in the TCPVars for tracking if SACKOK is set.

Reduces size by 16 bytes.

Ticket: #6938.
1 year ago
Victor Julien 2d5c381c3b decode/ipv4: prep for turning ip4h/ip6h into union
Store IPv4 decoder vars in a new Packet::l3 section in the packet.

Use inline functions instead of the often multi-layer macro's for
various IPv4 header getters.

Ticket: #6938.
1 year ago
Victor Julien 36f6e05155 counters: make tcp stats independent of flow, ssn
Counters depended on availability of flow and tcp session, meaning
that 2 memcaps could affect the counters.

Bug: #5017.
2 years ago
Philippe Antoine 1f066cbbe8 unittest: fix unneeded includes as per cppclean
Especially because there is conditional inclusion from a header
3 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.
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.
4 years ago
Jeff Lucovsky 1eeb96696b general: Cleanup bool usage 4 years ago
Jeff Lucovsky f8fef0dd05 decode/tcp: Improved handling of TFO options
This commit improves handling of TCP fast open options
- Option length must be in [6, 18]
- Option length must be an even value
5 years ago
Victor Julien 41d0dcae99 decode: cleanup packet properly on bad packets
In case of bad IPv4, TCP or UDP, the per packet ip4vars/tcpvars/udpvar
structures would not be cleaned up because the cleanup depends on the
'header' pointer being set, but the error handling would unset that.

This could mean these structures were already filled with values before
the error was detected. As packets were recycled, the next packet decoding
would use this unclean structure.

To make things worse these structures are part of unions. IPv4/IPv6 and
TCP/ICMPv4/ICMPv6 share the same memory location.

LibFuzzer+UBSAN found this both locally and in Oss-Fuzz:

decode-ipv6.c:654:9: runtime error: load of value 6, which is not a valid value for type 'bool'
    #0 0x6146f0 in DecodeIPV6 /src/suricata/src/decode-ipv6.c:654:9
    #1 0x617e96 in DecodeNull /src/suricata/src/decode-null.c:70:13
    #2 0x9dd8a4 in DecodePcapFile /src/suricata/src/source-pcap-file.c:412:9
    #3 0x4c8ed2 in LLVMFuzzerTestOneInput /src/suricata/src/tests/fuzz/fuzz_sigpcap.c:158:25
    #4 0x457e51 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
    #5 0x457575 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:470:3
    #6 0x459917 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:698:19
    #7 0x45a6a5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:830:5
    #8 0x448728 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:824:6
    #9 0x472552 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
    #10 0x7ff0d097b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #11 0x41bde8 in _start (/out/fuzz_sigpcap+0x41bde8)

Bug: #3496
5 years ago
Victor Julien f8aed4ce2d threading: change local packet queue logic
Previously each 'TmSlot' had it's own packet queue that was passed
to the registered SlotFunc as an argument. This was used mostly for
tunnel packets by the decoders and by defrag.

This patch removes that in favor of a single queue in the ThreadVars:
decode_pq. This is the non-locked version of the queue as this is
only a temporary store for handling packets within a thread.

This patch removes the PacketQueue pointer argument from the API.
The new queue can be accessed directly through the ThreadVars
pointer.
5 years ago
Victor Julien df74f34a62 decode/tcp: accept TCP fast open cookie request 6 years ago
Victor Julien fb26268c6b tcp: don't set event on empty SACK opt
TCP_OPT_INVALID_LEN was set if the opt len was 2. While useless
an empty SACK is not uncommon.

Seen on an iOS device talking to an Apple server.

Bug #3254.
6 years ago
Victor Julien 579cc9f02b const: constify decoder, app-layer, detect funcs 6 years ago
Victor Julien 8f8581beda decode/tcp: TCP fast open option decoding
Support both regular TFO and TFO as part of the experimental
options support.
6 years ago
Jeff Lucovsky 8e464530ef decode: Change return type of IPv4 and TCP options decode
The return value from the options decoder in TCP and IPv4 is ignored.
This commit changes the return type of the function to `void` and
modifies existing return points to return without a value.

When an error occurs, the packet state is being set to indicate whether
it's valid or not and the existing return value is never used.
6 years ago
Victor Julien cf37faff31 decode/tcp: rewrite options decoding to assist scan-build 7 years ago
Victor Julien 11be9bd971 mingw: add SCNtohl and SCNtohs macro's
On MinGW the result of ntohl needs to be casted to uint32_t and
the result of ntohs to uint16_t. To avoid doing this everywhere
add SCNtohl and SCNtohs macros.
8 years ago
Jason Ish b79a18ea15 tcp/udp: rename checksum functions for better meaning
The TCP/UDP checksum functions no longer just calculate
the checksum, they can validate as well as calculate so
use a more generic name.
8 years ago
Jason Ish f56428d996 tcp/udp: fix checksum validation when 0xffff
Issue:
https://redmine.openinfosecfoundation.org/issues/2041

One approach to fixing this issue to just validate the
checksum instead of regenerating it and comparing it. This
method is used in some kernels and other network tools.

When validating, the current checksum is passed in as an
initial argument which will cause the final checksum to be 0
if OK. If generating a checksum, 0 is passed and the result
is the generated checksum.
8 years ago
Victor Julien 2f0e0f17db flow: move flow handling into worker threads
Instead of handling the packet update during flow lookup, handle
it in the stream/detect threads. This lowers the load of the
capture thread(s) in autofp mode.

The decoders now set a flag in the packet if the packet needs a
flow lookup. Then the workers will take care of this. The decoders
also already calculate the raw flow hash value. This is so that
this value can be used in flow balancing in autofp.

Because the flow lookup/creation is now done in the worker threads,
the flow balancing can no longer use the flow. It's not yet
available. Autofp load balancing uses raw hash values instead.

In the same line, move UDP AppLayer out of the DecodeUDP module,
and also into the stream/detect threads.

Handle TCP session reuse inside the flow engine itself. If a looked up
flow matches the packet, but is a TCP stream starter, check if the
ssn needs to be reused. If that is the case handle it within the
lookup function. Simplies the locking and removes potential race
conditions.
9 years ago
Victor Julien 76c8c077c5 tcp: fix alignment issues with tcp timestamps 9 years ago
Jason Ish 796dd5223b tests: no longer necessary to provide successful return code
1 pass, 0 is fail.
9 years ago
Jason Ish 52983bf314 tests: convert all test to return 0 on failure, 1 on success 9 years ago
Victor Julien 38f67d88ea tcp: reduce TCP options storage in packets
Until now, the TCP options would all be stored in the Packet structure.
The commonly used ones (wscale, ts, sack, sackok and mss*) then had a
pointer to the position in the option array. Overall this option array
was large. About 360 bytes on 64bit systems. Since no part of the engine
would every access this array other than through the common short cuts,
this was actually just wasteful.

This patch changes the approach. It stores just the common ones in the
packet. The rest is gone. This shrinks the packet structure with almost
300 bytes.

* even though mss wasn't actually used
9 years ago
Victor Julien 1c0b4ee0ae counters: s/SCPerfCounterIncr/StatsIncr/g 10 years ago
Victor Julien e9b067c1eb counters: make increment call take threadvars
This hides the implementation from the caller.
10 years ago
Victor Julien 9a8bff7d96 counters: threadvars s/sc_perf_pca/perf_private_ctx/g 10 years ago
Victor Julien de034f1867 flow: prepare flow forced reuse logging
Most flows are marked for clean up by the flow manager, which then
passes them to the recycler. The recycler logs and cleans up. However,
under resource stress conditions, the packet threads can recycle
existing flow directly. So here the recycler has no role to play, as
the flow is immediately used.

For this reason, the packet threads need to be able to invoke the
flow logger directly.

The flow logging thread ctx will stored in the DecodeThreadVars
stucture. Therefore, this patch makes the DecodeThreadVars an argument
to FlowHandlePacket.
11 years ago
Eric Leblond b2c58b8d14 Set packet invalid flag during decoding.
This patch set a new value in pkt->flag to signal that a packet is
invalid during decoding. The patch has been obtained via a coccinelle
transformation.
12 years ago
Eric Leblond d4b7ecfbe3 decode: update API to return error
In some cases, the decoding is not possible and some really invalid
packet can be created. This is in particular the case of tunnel. In
that case, it is more interesting to forget about the tunneled
packet and only consider the original packet.

DecodeTunnel function is maked as warn_unused_result because it is
meaningful for the decoder to know if the underlying data were not
correct. And in this case, only focus detection on the content.
12 years ago
Victor Julien 49087f21e4 Optimizations to reduce branch misses 12 years ago
Eric Leblond c5bd04f102 unittest: recycle packet before exit
To avoid an issue with flow validation, we need to recycle the packet
before cleaning the flow.
12 years ago
Ken Steele 9c7b411a5d More PacketGetFromMalloc() to allocate packets. 12 years ago
Eric Leblond e176be6fcc Use unlikely for error treatment.
When handling error case on SCMallog, SCCalloc or SCStrdup
we are in an unlikely case. This patch adds the unlikely()
expression to indicate this to gcc.

This patch has been obtained via coccinelle. The transformation
is the following:

@istested@
identifier x;
statement S1;
identifier func =~ "(SCMalloc|SCStrdup|SCCalloc)";
@@

x = func(...)
... when != x
- if (x == NULL) S1
+ if (unlikely(x == NULL)) S1
13 years ago
Victor Julien 06b1d71032 Small optimizations to IPV4 and TCP header parsing. 14 years ago
Eric Leblond acf10525f6 doc: add decode group and related documentation. 14 years ago
Eric Leblond 7425bf5ca6 Rename some decode event structure and macro.
This patch renames DECODER_SET_EVENT, DECODER_ISSET_EVENT and some
other structures to ENGINE equivalent to take into account the fact
the event list is now related to all engines and not only to decoder.
14 years ago
Victor Julien 6aa551c558 Small optimizations to IPV4 and TCP header parsing. 14 years ago
Victor Julien 6fc075d4ae Add TCP packet SACK option decoding. 14 years ago
Victor Julien 778b92ef40 Make sure to only alloc a new pseudo packet once during ip defrag. 14 years ago
Eric Leblond 1db4aadd16 Supress usage of Packet declaration in tests.
For convenience, a massive usage of 'Packet p;' declaration has
been done in the tests function. Although this was completely
legal, this is not possible anymore because of the new Packet
allocation structure. This massive patch modifies all suricata
files to use a SCMalloc allocated pointer to Packet instead.

This patch has been done using coccinelle (http://coccinelle.lip6.fr)
which is a semantic patching tool. This ensures that things like call
to SCFree() should have not been forget because the semantic patch
explicitly forces the call to SCFree(p) before each return. With this
patch all unittests are running fine with a small and a big default
packet size.
15 years ago