Jeremy Sowden [Mon, 26 May 2025 17:19:03 +0000 (18:19 +0100)]
Use `NFPROTO_*` constants for protocol families
Netfilter has a set of `NFPROTO_*` constants for the protocol families that it
supports, in part because it supports protocols and pseudo-protocols that do not
have `PF_*` (and `AF_*`) constants. Currently, ulogd uses `AF_*` constants for
protocol families, because it does not support any families which do not have
`AF_*` constants. Switch to `NFPROTO_*` constants instead, so we can add ARP
support later.
In the IP2* filters, retain `AF_*` for address family variables.
Remove a stray semicolon.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
IP2STR and IP2BIN do all family checks inside the for-loop that converts the
address fields, whereas IP2HBIN does the checks once before the loop. Refactor
the former to do as the latter.
Also, move all the remaining contents of the for-loops, apart from the
`pp_is_valid` checks, into `ip2*` functions.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
`format_ipv6()` formats IPv6 addresses as hex-strings. However, sizing for the
output buffer is not done quite right.
The elements of the `ipbin_array` array in ulogd_filter_IP2BIN.c are sized using
a local macro, `IPADDR_LENGTH`, which is defined as 128, the number of bits in
an IPv6 address; this is much larger than necessary.
Define an appropriate macro and use that instead.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 26 Mar 2025 23:07:49 +0000 (00:07 +0100)]
nfct: add network namespace support
Allow the plugin to fetch data from a different network namespace. This
is possible by changing the network namespace before opening the netlink
socket, and immediately changing back to the original network namespace
once the socket is open. The number of nfct_open usages here warranted a
dedicated wrapper function.
If changing back to the original network namespace fails, ulogd will
log an error, but continue to run in a different network namespace than
it was started in, which may cause unexpected behaviour. But I don't see
a way to properly "escalate" it such that ulogd aborts entirely.
Also slightly adjust the error log messages to specify which socket
failed to open.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 26 Mar 2025 23:06:18 +0000 (00:06 +0100)]
ulogd: add linux namespace helper
The new namespace helper provides an internal stable interface for
plugins to use for switching various linux namespaces. Currently only
network namespaces are supported/implemented, but can easily be extended
if needed. autoconf will enable it automatically if the required symbols
are available. If ulogd is compiled without namespace support, the
functions will simply return an error, there is no need for conditional
compilation or special handling in plugin code.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Sun, 20 Apr 2025 17:20:20 +0000 (18:20 +0100)]
IP2STR: correct address buffer size
The elements of the `ipstr_array` array are `IPADDR_LENGTH` bytes long where
`IPADDR_LENGTH` is a local macro defined as 128. However, this is the number of
bits in an IPv6 address, but the elements of `ipstr_array` only need to be big
enough to be used for the output of `inet_ntop`. Use the standard
`INET6_ADDRSTRLEN` macro instead.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 29 Mar 2025 11:08:55 +0000 (12:08 +0100)]
ulogd: remove libipulog
The ULOG target was removed from linux kernel with 7200135bc1e6
("netfilter: kill ulog targets") aka v3.17, so remove the userspace
library for it. libnetfilter_log provides the same functionality for
NFLOG, and also a compatibility layer to use NFLOG through the libipulog
api.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 29 Mar 2025 11:07:06 +0000 (12:07 +0100)]
ulog: remove input plugin
The ULOG target was removed from the linux kernel with 7200135bc1e6
("netfilter: kill ulog targets") aka v3.17, so remove the input plugin
for it. It's successor NFLOG should be used instead, which has its own
input plugin.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Tue, 25 Mar 2025 00:26:16 +0000 (01:26 +0100)]
nfct: add flow end timestamp on hashtable purge
In polling mode during normal operation, as well as in event mode with
hashtable when an overrun occurs, the hashtable is fully re-synced
against conntrack. When removing flows from the hashtable that are no
longer in conntrack, there is no way to get the actual end timestamp of
the flow from conntrack because it is already gone. Since the last
conntrack data in the hashtable for these flows will never contain an
end timestamp in this case, set_timestamp_from_ct() will always fall
back to using the current time, aka when the plugin determines that the
flow disappeared from conntrack. That is only an approximation, but
should be good enough; and certainly more accurate than no end timestamp
at all.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Tue, 25 Mar 2025 00:24:04 +0000 (01:24 +0100)]
nfct: fix counter-reset without hashtable
In event mode the hashtable is optional, and sending SIGUSR2 to ulogd will
call get_ctr_zero().
The dump_reset_handler will try to update the hashtable regardless of
whether it is used (and thus initialized), which results in a segfault
if it isn't. Instead just short-circuit the handler, and skip any
further result processing because it's not used in this case anyway.
All flow counters in conntrack are reset regardless of the return value
of the handler/callback.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 12 Mar 2025 19:09:36 +0000 (20:09 +0100)]
ulogd: raise error on unknown config key
Until a6fbeb96e889 ("new configuration file syntax (Magnus Boden)")
this was already caught, and the enum member is still present.
Check if the for loop worked throught the whole array without hitting a
matching config option, and return with the unknown key error code.
Because there is no existing config_entry struct with that unknwon key
to use with the established config_errce pointer, allocate a new struct.
This potentially creates a memory leak if that config_entry is never
freed again, but for me that is acceptable in this rare case.
Since the memory allocation for the struct can fail, also reuse the old
out-of-memory error to indicate that.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 12 Mar 2025 14:55:55 +0000 (15:55 +0100)]
ulogd: provide default configure implementation
Provide a default implementation for the configure hook which simply
calls ulogd_parse_configfile(), so simple plugins only need to provide
the config_keyset. This also triggers an "unknown key" error if a
plugin defines no config_keyset (aka it has no options), but the config
file contains directives for it.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 12 Mar 2025 14:55:16 +0000 (15:55 +0100)]
ulogd: improve integer option parsing
The `value` union member in `struct config_entry` is declared as `int`
since basically the beginning in e07722e46001 ("config stuff added").
The parsing was switched from the original `atoi()` in 015849995f7f
("Fix hexadecimal parsing in config file") to `strtoul()`.
Switch the function for parsing to the signed `strtol()` variant since
the result will be stored in a signed int, and it makes sense to support
negative numbers. Detect when `strtol()` does not properly consume the
whole argument and return a new format error. Also check the numerical
value to make sure the signed int does not overflow, in which case
a new range error is returned.
Unfortunately there is no `strtoi()` which would do the proper range
check itself, so the intermediate `long` and range-check for `int` is
required. I also considered changing the `value` union member from
`int` to `long`, which would make it possible to use the parsed value
as-is. But since this is part of the api towards plugins (including
third party) such a potentially breaking change felt unwarranted. This
also means that still only 16bit integer values are *guaranteed* to
work, although most platforms use bigger widths for int.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 12 Mar 2025 14:53:46 +0000 (15:53 +0100)]
all: use config_parse_file function in all plugins
Replace all usages of `config_parse_file()` in plugins with the new
`ulogd_parse_configfile()` function, adding error handling where it was
missing. I used the same codestyle as the surrounding code, which varies
between plugins.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Wed, 12 Mar 2025 14:52:29 +0000 (15:52 +0100)]
ulogd: add ulogd_parse_configfile public function
Provide a new function `ulogd_parse_configfile()` in the public
interface, which wraps `parse_config_file()` to parse a section of the
config file and communicates found errors to the user. It can be used
as a drop-in replacement because arguments and return value are
compatible.
This relieves plugins of the need to translate the individual error
codes to human readable messages, and plugins are mostly interested if
there is any error, not what specific error.
This reuses the existing `parse_conffile()` function with slight
adjustments.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 8 Mar 2025 21:30:05 +0000 (22:30 +0100)]
nfct: add icmpv6
Add two new dedicated fields to provide the ICMPv6 code and type. While
libnetfilter_conntrack uses the same attribute for both ICMPv4 and v6,
there are no version-agnostic ICMP IEs in IPFIX.
The fields are annotated with the appropriate IPFIX metadata, which is
currently not actually used anywhere. You may call it consistency,
future-proofing or cargo-culting.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 8 Mar 2025 22:36:33 +0000 (23:36 +0100)]
ulogd: ignore malformed config directives
When a config directive is provided with a malformed argument (e.g.
`loglevel="1`), then the call to get_word() returns NULL and `wordbuf`
is left unchanged aka still contains the directive name. Unlike the
previous calls to get_word(), the return value is not checked here, and
processing continues with `args` pointing to the still unchanged
`wordbuf`. So `loglevel="1` is effectively parsed as
`loglevel=loglevel`.
Instead if no valid argument is found, ignore the directive and log a
warning.
Due to the way get_word() is implemented, this unfortunately will report
an empty argument (e.g. `loglevel=`) as malformed as well. Ideally that
should behave the same as `loglevel=""`, but I found no nice way to
achieve that. An empty argument is only useful in rare cases, so
treating it as malformed should be fine for now. That's still way better
than the previous broken "name as value" behaviour.
Fixes: e88384d9d5a1 ("added new generic get_word() function to do better parsing") Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 8 Mar 2025 21:37:28 +0000 (22:37 +0100)]
ulogd: ignore private data on plugin stop
When deciding whether to call the stop hook of a plugin instance, only
two things are relevant: If the plugin actually has a stop hook defined,
and if the plugin instance is still used in a different stack. The
private data of a plugin instance is opaque to ulogd, so its size or
content are irrelevant to the stop-hook decision. And in the same vein
should ulogd never write to it.
The one-null-byte write could previously lead to an out-of-bounds write
on plugins with a stop hook and zero-size private data.
Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sun, 16 Feb 2025 14:05:01 +0000 (15:05 +0100)]
ipfix: re-arm send timer
I am not sure what this timer was meant to do. My best guess is to send
an ipfix message every second if there is data, as to make sure reports
go out in a timely manner. Otherwise a message is only sent when adding
another flow would go past the max mtu, which may take a while if there
isn't much (filtered) traffic.
Timers in ulogd only fire once; if they should fire repeatedly (which I
guess was the intention here), they need to be re-armed in the callback.
Because that wasn't done, the timer only fired once 1 second after
starting the plugin (when there is unlikely any data yet), and then
never again.
The timer is now re-armed in the callback to make it fire repeatedly
every second(ish). A macro is used to make sure the initial and re-arm
time interval is the same.
Corubba Smith [Sun, 16 Feb 2025 14:03:51 +0000 (15:03 +0100)]
gprint: fix comma after ip addresses
Do the same as the oprint plugin: let inet_ntop() write to a temporary
buffer, and then write that buffer content and the trailing comma to the
actual output buffer in one go.
Fixes: f04bf6794d11 ("gprint, oprint: use inet_ntop to format ip addresses") Signed-off-by: Corubba Smith <corubba@gmx.de> Signed-off-by: Florian Westphal <fw@strlen.de>
Corubba Smith [Sat, 8 Mar 2025 21:27:56 +0000 (22:27 +0100)]
nfct: fix calloc argument order
The first argument to calloc() is the number of elements, the second is
the size of a single element. Having the arguments switched shouldn't
make any difference during runtime, but GCC warns about it when using
-Wcalloc-transposed-args [0].
Jeremy Sowden [Mon, 21 Aug 2023 19:42:36 +0000 (20:42 +0100)]
sqlite3: insert ipv6 addresses as null rather than garbage
Currently, the plug-in assumes that all IP addresses are 32-bit ipv4
addresses, so ipv6 addresses get truncated and inserted as garbage.
Insert nulls instead.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Mon, 21 Aug 2023 19:42:33 +0000 (20:42 +0100)]
gprint, oprint: use inet_ntop to format ip addresses
Replace hand-rolled ipv4-only formatting code in order to be able to
support ipv6 addresses. This also changes the byte-order expected by
oprint from HBO to NBO.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Mon, 21 Aug 2023 19:42:30 +0000 (20:42 +0100)]
raw2packet_BASE: store ARP address values as integers
Keys of type `ULOGD_RET_IPADDR` may be ipv4 or ipv6. ARP protocol
addresses are 32-bits (i.e., ipv4). By using `okey_set_u32` we keep
track of the size and allow downstream plug-ins to handle them
correctly.
Reported-by: Robert O'Brien <robrien@foxtrot-research.com> Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Mon, 21 Aug 2023 19:42:29 +0000 (20:42 +0100)]
printpkt, raw2packet_BASE: keep gateway address in NBO
Everywhere else ipv4 addresses are left in NBO until output. The only
exception is the IP2HBIN filter, which is explicitly intended to convert
from NBO to HBO.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Kyuwon Shim [Thu, 9 Mar 2023 01:24:47 +0000 (14:24 +1300)]
ulogd2: avoid use after free in unregister on global ulogd_fds linked list
Invalid read of size 4
at 0x405F60: ulogd_unregister_fd (select.c:74)
by 0x4E4E3DF: ??? (in /usr/lib/ulogd/ulogd_inppkt_NFLOG.so)
by 0x405003: stop_pluginstances (ulogd.c:1335)
by 0x405003: sigterm_handler_task (ulogd.c:1383)
by 0x405153: call_signal_handler_tasks (ulogd.c:424)
by 0x405153: signal_channel_callback (ulogd.c:443)
by 0x406163: ulogd_select_main (select.c:105)
by 0x403CF3: ulogd_main_loop (ulogd.c:1070)
by 0x403CF3: main (ulogd.c:1649)
Problem is that ulogd_inppkt_NFLOG.c::stop() calls ulogd_unregister_fd()
which does llist_del(). This llist_del may touch ->prev pointer.
As the list element is in private data, we cannot do this llist_del
from stop_pluginstances().
Therefore, the free() process moved location after finishing ulogd_unregister_fd().
Jeremy Sowden [Thu, 16 Mar 2023 11:07:54 +0000 (11:07 +0000)]
pcap: prevent crashes when output `FILE *` is null
If ulogd2 receives a signal it will attempt to re-open the pcap output
file. If this fails (because the permissions or ownership have changed
for example), the FILE pointer will be null and when the next packet
comes in, the null pointer will be passed to fwrite and ulogd will
crash.
Instead, assign the return value of `fopen` to a local variable, and
only close the existing stream if `fopen` succeeded.
Jeremy Sowden [Thu, 16 Mar 2023 11:07:53 +0000 (11:07 +0000)]
pcap: simplify opening of output file
Instead of statting the file, and choosing the mode with which to open
it and whether to write the PCAP header based on the result, always open
it with mode "a" and _then_ stat it. This simplifies the flow-control
and avoids a race between statting and opening.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Florian Westphal <fw@strlen.de>
Jeremy Sowden [Tue, 13 Dec 2022 11:19:51 +0000 (11:19 +0000)]
build: fix pgsql fall-back configuration of CFLAGS
When using mysql_config and pcap_config to configure `CFLAGS`, one
requests the actual flags:
$mysql_config --cflags
$pcap_config --cflags
By constrast, when using pg_config, one requests the include-directory:
$pg_config --includedir
Therefore, the `-I` option has to be explicitly added.
Fixes: 20727ab8b9fc ("build: use pkg-config or pg_config for libpq") Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 29 Nov 2022 21:11:27 +0000 (21:11 +0000)]
pgsql: correct `ulog2.ip_totlen` type
The types of `ip_totlen` in the `ulog` view and the `INSERT_IP_PACKET_FULL`
function are `integer`, but the column in the `ulog2` table is `smallint`. The
"total length" field of an IP packet is an unsigned 16-bit integer, whereas
`smallint` in PostgreSQL is a signed 16-bit integer type. Change the type of
`ulog2.ip_totlen` to `integer`.
Jeremy Sowden [Sat, 3 Dec 2022 19:02:12 +0000 (19:02 +0000)]
db: fix back-log capacity checks
Hitherto, when adding queries to the back-log, the memory usage has been
incremented and decremented by the size of the query structure and the
length of the SQL statement, `sizeof(struct db_stmt) + len`. However,
when checking whether there is available capacity to add a new query,
the struct size has been ignored. Amend the check to include the struct
size, and also account for the NULL that terminates the SQL.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The arrays are indexed by subtracting `START_KEY` from the enum value of
the key currently being processed: `hwmac_str[okey - START_KEY]`.
However, this means that the last key (`KEY_MAC_ADDR` in this example)
will run off the end of the array. Increase the size of the arrays.
In the case of `IP2BIN` and `IP2HBIN`, there is no overrun, but only
because they use the wrong upper bound when looping over the keys, and
thus don't assign a value to the last key. Correct the bound.
Jeremy Sowden [Sat, 3 Dec 2022 19:02:09 +0000 (19:02 +0000)]
ulogd: fix parse-error check
If `config_parse_file` returns `-ERRTOOLONG`, `config_errce` may be
`NULL`. However, the calling function checks whether
`config_errce->key` is `NULL` instead.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Sun, 9 Jan 2022 11:57:47 +0000 (11:57 +0000)]
build: use pkg-config or pcap-config for libpcap
Recent versions of libpcap support pkg-config. Older versions provide a
pcap-config script. Use pkg-config if available, otherwise fall back to
pcap-config.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Sun, 9 Jan 2022 11:57:46 +0000 (11:57 +0000)]
build: use pkg-config or mysql_config for libmysqlclient
Recent versions of mariadb and mysql support pkg-config. Older versions
provide a mysql_config script. Use pkg-config if available, otherwise
fall back to mysql_config.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Sun, 9 Jan 2022 11:57:44 +0000 (11:57 +0000)]
build: use `--enable-XYZ` options for output plugins
Currently, we use `AC_ARG_WITH` for output plugins. However, this is
not consistent with the input plugins, which use `AC_ARG_ENABLE`, and in
some cases (dbi, mysql, pgsql) the macro calls in configure.ac conflict
with others in acinclude.m4. Use `AC_ARG_ENABLE` instead and change the
name of the option for the JSON plugin from `jansson` to `json`.
Fixes: 51ba7aec8951 ("Fix automagic support of dbi, pcap and sqlite3") Fixes: c61c05c2d050 ("configure.ac: Add --without-{mysql,pgsql}") Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Wed, 5 Jan 2022 22:37:21 +0000 (22:37 +0000)]
output: SQLITE3: remove unused variable
There's local variable left over from a previous tidy-up. Remove it.
Fixes: 67b0be90f16f ("output: SQLITE3: improve mapping of fields to DB columns") Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:56:00 +0000 (10:56 +0000)]
output: IPFIX: remove compiler attribute macros
The ipfix.h header includes three macros which expand to compiler attributes.
Presumably, at some point the definitions were one branch of an if-else
preprocessor conditional where the definitions in the other branch expanded to
nothing. This is no longer the case. Only one of the macros (`__packed`) is
used and the raw attribute is used elsewhere in the code-base. Remove the
macros.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:57 +0000 (10:55 +0000)]
output: JSON: fix possible leak in error-handling.
The `realloc` extending the buffer containing the JSON to allow us to
insert a final new-line may fail. Therefore, we need to assign the
return-value to a temporary variable or we will not able to free the
existing buffer on error.
Use the correct type for `buflen`.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:56 +0000 (10:55 +0000)]
output: JSON: increase time-stamp buffer size
The output buffer for date-times is of sufficient size provided that we
don't get oversized integer values for any of the fields, which is a
reasonable assumption. However, the compiler complains about possible
truncation, e.g.:
ulogd_output_JSON.c:314:65: warning: `%06u` directive output may be truncated writing between 6 and 10 bytes into a region of size between 0 and 18
ulogd_output_JSON.c:313:25: note: `snprintf` output between 27 and 88 bytes into a destination of size 38
Fix the warnings by increasing the buffer size.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:55 +0000 (10:55 +0000)]
output: JSON: fix output of GMT offset
The compiler has two sets of complaints. Firstly, `t->tm_gmtoffset` is
a `long int`, but it is being passed to `abs`, which leads to warnings
such as:
ulogd_output_JSON.c:308:34: warning: absolute value function `abs` given an argument of type `long int` but has parameter of type `int` which may cause truncation of value
Secondly, it can't verify that the hour value derived from the offset
will in fact fit into `%02d`, thus:
ulogd_output_JSON.c:306:37: warning: `%02d` directive output may be truncated writing between 2 and 6 bytes into a region of size 5
To remedy these, we now mod the offset by 86,400 and assign it to an `int`
before deriving the hour and minute values.
We also change the format-specifier for the hour value to `%+03d` which
causes a sign to be printed even if the value is positive, thus allowing
us not to specify the sign explicitly and to drop the `abs` call for the
hour value.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:54 +0000 (10:55 +0000)]
db: simplify initialization of ring-buffer
Currently, `strncpy` is used to copy the SQL statement to the ring
buffer, passing the length of the source string, which leads gcc to
complain:
../../util/db.c:231:25: warning: `strncpy` specified bound depends on the length of the source argument
In fact, the ring buffer is sized to be a multiple of the size of the
SQL buffer, and the SQL is simply copied multiple times at increasing
offsets, so use `strcpy` instead.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:53 +0000 (10:55 +0000)]
db: improve mapping of input-keys to DB columns
Currently, we copy the key-name to a buffer, iterate over it to replace
the full-stops with underscores, using `strchr` from the start of the
buffer on each iteration, then append the buffer to the SQL statement.
Apart from the inefficiency, `strncpy` is used to do the copies, which
leads gcc to complain:
../../util/db.c:118:25: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Furthermore, the buffer is one character too short and so there is the
possibility of overruns.
Instead, append the key-name directly to the statement using `sprintf`,
and run `strchr` from the last underscore on each iteration.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:52 +0000 (10:55 +0000)]
db: improve formatting of insert statement
`sql_createstmt` contains a variable `stmt_val` which points to the end
of the SQL already written, where the next chunk should be appended.
Currently, this is assigned after every write:
However, since `sprintf` returns the number of bytes written, increment
`stmt_val` by the return-value of `sprintf` in order to avoid the
repeated `strlen` calls.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
`sqlite3_createstmt` returns non-zero on error, but the return-value was
being ignored. Change the calling code to check the return-value, log
an error message and propagate the error.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
output: SQLITE3: improve mapping of fields to DB columns
Currently, we derive a field-name by replacing all the underscores in a
DB column-name with full-stops and use the field-name to find the
matching input-key. However, every time we create a new insert SQL
statement, we derive the column-names by copying the field-names to a
buffer, replacing all the full-stops with underscores, and then
appending the buffer containing the column-name to the one containing
the statments.
Apart from the inefficiency, `strncpy` is used to do the copies, which
leads gcc to complain:
ulogd_output_SQLITE3.c:234:17: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Instead, leave the underscores in the field-name, but copy it once to a
buffer in which the underscores are replaced and use this to find the
input-key.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:49 +0000 (10:55 +0000)]
output: SQLITE3: improve mapping of DB columns to fields
Currently, we copy the column-name to a buffer, iterate over it to
replace the underscores with full-stops, using `strchr` from the start
of the buffer on each iteration, then copy the buffer to the field's
`name` member.
Apart from the inefficiency, `strncpy` is used to do the copies, which
leads gcc to complain:
ulogd_output_SQLITE3.c:341:17: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Furthermore, the buffer is not initialized, which means that there is
also a possible buffer overrun if the column-name is too long, since
`strncpy` will not append a NUL.
Instead, copy the column-name directly to the field using `snprintf`,
and run `strchr` from the last underscore on each iteration.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:48 +0000 (10:55 +0000)]
output: SQLITE3: improve formatting of insert statement
`sqlite3_createstmt` contains a variable `stmt_pos` which points to the
end of the SQL already written, where the next chunk should be appended.
Currently, this is assigned after every write:
However, since `sprintf` returns the number of bytes written, increment
`stmt_pos` by the return-value of `sprintf` in order to avoid the
repeated `strlen` calls.
Pablo mangled this original patch to add this chunk at the end of this
patch (originally submitted as a conversion to use strcpy).
+ for (i = 0; i < cols - 1; i++)
+ stmt_pos += sprintf(stmt_pos, "?,");
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Sat, 4 Dec 2021 20:56:00 +0000 (20:56 +0000)]
build: bump libnetfilter_log dependency
Recent changes to add conntrack info to the NFLOG output plug-in rely on
symbols only present in the headers provided by libnetfilter-log v1.0.2:
CC ulogd_inppkt_NFLOG.lo
ulogd_inppkt_NFLOG.c: In function 'build_ct':
ulogd_inppkt_NFLOG.c:346:34: error: 'NFULA_CT' undeclared (first use in this function); did you mean 'NFULA_GID'?
if (mnl_attr_get_type(attr) == NFULA_CT) {
^~~~~~~~
NFULA_GID
ulogd_inppkt_NFLOG.c:346:34: note: each undeclared identifier is reported only once for each function it appears in
ulogd_inppkt_NFLOG.c: In function 'start':
ulogd_inppkt_NFLOG.c:669:12: error: 'NFULNL_CFG_F_CONNTRACK' undeclared (first use in this function); did you mean 'NFULNL_CFG_F_SEQ'?
flags |= NFULNL_CFG_F_CONNTRACK;
^~~~~~~~~~~~~~~~~~~~~~
NFULNL_CFG_F_SEQ
Jeremy Sowden [Tue, 30 Nov 2021 10:55:47 +0000 (10:55 +0000)]
output: SQLITE3: fix memory-leak in error-handling
When mapping DB column names to input-keys, if we cannot find a key to
match a column, the newly allocated `struct field` is leaked. Free it,
and log an error message.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:46 +0000 (10:55 +0000)]
output: SQLITE3: fix possible buffer overruns
There is a an off-by-one error in the size of some of the buffers used
to hold key-names. The maximum length of a name is `ULOGD_MAX_KEYLEN`,
and so declare the buffers with size `ULOGD_MAX_KEYLEN + 1`.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:45 +0000 (10:55 +0000)]
output: PGSQL: fix non-`connstring` configuration of DB connection
In `open_db_pgsql`, we test whether various config-settings are defined
by comparing their string values to `NULL`. However, the `u.string`
member of `struct config_entry` is an array, not a pointer, so it is
never `NULL`. Instead, check whether the string is empty.
Use a pointer to the end of the `connstr` buffer and `sprintf`, rather
than repeated `strcat`s.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:44 +0000 (10:55 +0000)]
output: PGSQL: improve mapping of DB columns to input-keys
Currently, we copy the column-name to a buffer, iterate over it to
replace the underscores with full-stops, using `strchr` from the start
of the buffer on each iteration, then copy the buffer to the input-key's
`name` member.
Apart from the inefficiency, `strncpy` is used to do the copies, which
leads gcc to complain:
ulogd_output_PGSQL.c:204:17: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Furthermore, the buffer is not initialized, which means that there is
also a possible buffer overrun if the column-name is too long, since
`strncpy` will not append a NUL.
Instead, copy the column-name directly to the input-key using
`snprintf`, and run `strchr` from the last underscore on each iteration.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:43 +0000 (10:55 +0000)]
output: MYSQL: improve mapping of DB columns to input-keys
Currently, we copy the column-name to a buffer, iterate over it to
replace the underscores with full-stops, using `strchr` from the start
of the buffer on each iteration, then copy the buffer to the input-key's
`name` member.
Apart from the inefficiency, `strncpy` is used to do the copies, which
leads gcc to complain:
ulogd_output_MYSQL.c:149:17: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Furthermore, the buffer is not initialized, which means that there is
also a possible buffer overrun if the column-name is too long, since
`strncpy` will not append a NUL.
Instead, copy the column-name directly to the input-key using
`snprintf`, and run `strchr` from the last underscore on each iteration.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:42 +0000 (10:55 +0000)]
output: DBI: fix configuration of DB connection
In `open_db_dbi`, we test whether various config-settings are defined
by comparing their string values to `NULL`. However, the `u.string`
member of `struct config_entry` is an array, not a pointer, so it is
never `NULL`. Instead, check whether the string is empty.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:41 +0000 (10:55 +0000)]
output: DBI: fix NUL-termination of escaped SQL string
On error, `dbi_conn_quote_string_copy` returns zero. In this case, we
need to set `*dst` to NUL. Handle a return-value of `2` as normal
below. `1` is never returned.
Replace `strncpy` with `memcpy`: using `strncpy` is nearly always a
mistake, and we don't need its special behaviour here.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:40 +0000 (10:55 +0000)]
output: DBI: improve mapping of DB columns to input-keys
Currently, we copy the column-name to a buffer, iterate over it to
replace the underscores with full-stops, using `strchr` from the start
of the buffer on each iteration, iterate over it a second time to
lower-case all letters, and finally copy the buffer to the input-key's
`name` member.
In addition to being inefficient, `strncpy` is used to do the copies,
which leads gcc to complain:
ulogd_output_DBI.c:160:17: warning: `strncpy` output may be truncated copying 31 bytes from a string of length 31
Furthermore, the buffer is not initialized, which means that there is
also a possible buffer overrun if the column-name is too long, since
`strncpy` will not append a NUL.
Instead, copy the column-name directly to the input-key using
`snprintf`, and then iterate over it once to replace underscores and
lower-case letters.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:38 +0000 (10:55 +0000)]
input: UNIXSOCK: prevent unaligned pointer access
`struct ulogd_unixsock_packet_t` is packed, so taking the address of its
`struct iphdr payload` member may yield an unaligned pointer value. We
only actually dereference the pointer to get the IP version, so replace
the pointer with a version variable and elsewhere use `pkt.payload`
directly.
Remove a couple of stray semicolons.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:36 +0000 (10:55 +0000)]
input: UNIXSOCK: remove stat of socket-path
When creating the UNIX socket, there is a TOCTOU race between the
stat(2) and bind(2) calls, and if the path is already bound, the bind(2)
call will fail in any case. Remove the stat(2) call.
Tidy up a couple of error message.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Jeremy Sowden [Tue, 30 Nov 2021 10:55:35 +0000 (10:55 +0000)]
filter: PWSNIFF: replace malloc+strncpy with strndup
There are a couple of instances of allocating memory with `malloc`,
followed by copying a string to it with `strncpy` and adding an explicit
assignment of `\0` to terminate the string. Replace them with
`strndup`.
Add an enum to name indices of output keys.
Signed-off-by: Jeremy Sowden <jeremy@azazel.net> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
projects / thirdparty / ulogd2.git / log
