Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Next/605/20220411/v1 #7227

Merged
merged 3 commits into from
Apr 11, 2022
Merged

Conversation

inashivb
Copy link
Member

victorjulien and others added 3 commits April 11, 2022 13:09
Since:

9551cd0 ("threading: don't pass locked flow between threads")

`MoveToWorkQueue()` unconditionally unlocks the flow. This allows simpler
locking handling, including of tcp reuse flows.

The simpler logic also fixes a scenario where TCP reuse flows got "unlocked"
twice, once in `FlowGetFlowFromHash()` and once in `MoveToWorkQueue()`.

Bug: OISF#5248.
Coverity: 1494354.
(cherry picked from commit 57533d3)
If there is a space following a keyword that does not expect a value,
the rule fails to load due to improper value evaluation.
e.g. Space after "set" command
alert http any any -> any any (http.user_agent; dataset:set  ,ua-seen,type string,save datasets.csv; sid:1;)

gives error
[ERRCODE: SC_ERR_UNKNOWN_VALUE(129)] - dataset action "" is not supported.

Fix this by handling values correctly for such cases.

(cherry picked from commit 6d2a2a0)
(cherry picked from commit 7366396)
@inashivb
Copy link
Member Author

@catenacyber is the fuzz failure the one fixed by #7221?

@inashivb inashivb marked this pull request as ready for review April 11, 2022 09:32
@inashivb inashivb requested a review from a team as a code owner April 11, 2022 09:32
@catenacyber
Copy link
Contributor

is the fuzz failure the one fixed by #7221?

The fuzz failure should be fixed by one of the commits of #7221 indeed

@victorjulien victorjulien merged commit 0ddf393 into OISF:master-6.0.x Apr 11, 2022
@inashivb inashivb deleted the next/605/20220411/v1 branch April 11, 2022 15:37
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
The initdata argument to OutputFlowThreadInit was always NULL, remove
it. Internally the ThreadInit functions still get initdata, but this
is the data provided when that logging instance was registered.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
The use of OutputCtx as the data type for initdata was leaking Eve
submodule logic into the low level flow logger. Instead use void *, as
the flow logging module is not concerned with the type of data here.

Also document this initdata parameter.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
The callback, ThreadExitPrintStats is not used in the flow loggers.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
A library/plugin user wanting to register a custom flow logger must
include "output-flow.h", however that depends on some other includes.
One train of thought with respect to include files in libraries, is
that they should include all their dependencies on behalf of the
user. To make a custom flow logger just a little easier, include
"flow.h" and "decode.h".

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
The use of OutputCtx as the data type for initdata was leaking Eve
submodule logic into the low level packet logger. Instead use void *,
as the packet logging module is not concerned with the type of data
here.

Also document this initdata parameter.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
PgsqlTransactionState has a variant named "Init" which is a little too
generic to export to C. Fortunately this method doesn't need to be
exposed to C, instead remove it as it was only called by
rs_pgsql_tx_get_alstate_progress which also doesn't need to be public
or expose to C.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
This allows the header to be used without including other headers this
one depends on.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Loggers need an ID uniquely identify them for profiling. To help with
loggers registered at runtime (library, plugins), provide a
LOGGER_USER that can be used. It won't provide per-logger details if
they have more than one, but will provide a total for all their
registered loggers.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Add an example custom logger that hooks into the low level packet and
flow logging callbacks.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
The ThreadExitPrintStats callback was never being used, remove.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Rename OutputRegisterPacketLogger to SCOutputRegisterPacketLogger as
its part of the public API and document its parameters.

Comment on the other functions in the header that they are part of the
internal API.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Rename OutputRegisterFlowLogger to SCOutputRegisterFlowLogger and
document in the header file.

Mark other functions in the header file as part of the internal API.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Use the extending/output section to introduce the low level logging
API.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Using OutputCtx results in the low level output-tx packet logger being
aware of Suricata's higher level loggers that use OutputCtx, for the
low level logger this is purely opaque data that may not be an
OutputCtx for custom loggers.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Remove the callback to print stats on thread exit.  The counter value
was never being used and this helps us get rid of this callback
altogether as their is only one other usage of it.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Remove the ThreadExitPrintStats callback, this is the only logger that
was actually using it, and this logger is marked for deprecation. This
allows us to remove the callback from the registration signature.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
After removing the exit stats function from log-tlslog, this callback
is no longer used.

Ticket: OISF#7227
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
jasonish added a commit to jasonish/suricata that referenced this pull request Aug 30, 2024
Required to properly resolve the types in the header without depending
on includes coming before it in C files.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
The callback, ThreadExitPrintStats is not used in the flow loggers.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
A library/plugin user wanting to register a custom flow logger must
include "output-flow.h", however that depends on some other includes.
One train of thought with respect to include files in libraries, is
that they should include all their dependencies on behalf of the
user. To make a custom flow logger just a little easier, include
"flow.h" and "decode.h".

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
The use of OutputCtx as the data type for initdata was leaking Eve
submodule logic into the low level packet logger. Instead use void *,
as the packet logging module is not concerned with the type of data
here.

Also document this initdata parameter.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
PgsqlTransactionState has a variant named "Init" which is a little too
generic to export to C. Fortunately this method doesn't need to be
exposed to C, instead remove it as it was only called by
rs_pgsql_tx_get_alstate_progress which also doesn't need to be public
or expose to C.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
This allows the header to be used without including other headers this
one depends on.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Loggers need an ID uniquely identify them for profiling. To help with
loggers registered at runtime (library, plugins), provide a
LOGGER_USER that can be used. It won't provide per-logger details if
they have more than one, but will provide a total for all their
registered loggers.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Add an example custom logger that hooks into the low level packet and
flow logging callbacks.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
The ThreadExitPrintStats callback was never being used, remove.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Rename OutputRegisterPacketLogger to SCOutputRegisterPacketLogger as
its part of the public API and document its parameters.

Comment on the other functions in the header that they are part of the
internal API.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Rename OutputRegisterFlowLogger to SCOutputRegisterFlowLogger and
document in the header file.

Mark other functions in the header file as part of the internal API.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Use the extending/output section to introduce the low level logging
API.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Using OutputCtx results in the low level output-tx packet logger being
aware of Suricata's higher level loggers that use OutputCtx, for the
low level logger this is purely opaque data that may not be an
OutputCtx for custom loggers.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Remove the callback to print stats on thread exit.  The counter value
was never being used and this helps us get rid of this callback
altogether as their is only one other usage of it.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Remove the ThreadExitPrintStats callback, this is the only logger that
was actually using it, and this logger is marked for deprecation. This
allows us to remove the callback from the registration signature.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
After removing the exit stats function from log-tlslog, this callback
is no longer used.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Required to properly resolve the types in the header without depending
on includes coming before it in C files.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Rename OutputRegisterTxLogger to SCOutputRegisterTxLogger to make it
part of the public API as well as document.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Using OutputCtx leaks a higher level abstraction into the low level
logger.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Avoids leaking a higher level abstraction into a low level logger.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Avoids leaking a higher level abstraction into a low level logger.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Prefix the registration function and types with "SC", and add function
documentation.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Rename OutputRegisterFileLogger to SCOutputRegisterFileLogger, add
function documentation and include in userguide.

Ticket: OISF#7227
ct0br0 pushed a commit to ct0br0/suricata that referenced this pull request Sep 3, 2024
Prefix registration function and pointer function type with SC, as
well as document.

Ticket: OISF#7227
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants