Merge pull request #16947 from keszybz/socket-parsing-rework

Socket parsing rework

Merge pull request #17003 from yuwata/conf-parser-downgrade-log-level

tree-wide: downgrade log level in conf-parsers

Merge pull request #16986 from yuwata/network-fix-routing-policy-rule-issue-16784

network: fix routing policy rule issue

fs-util,tmpfiles: fix error handling of fchmod_opath()

When 4dfaa528d45 was first commited its callers relied on `errno` instead of the
return value for error reporting. Which worked fine, since internally
under all conditions base were set — even if ugly and not inline with
our coding style. Things then got broken in
f8606626ed3c2582e06543550d58fe9886cdca5f where suddenly additional
syscalls might end up being done in the function, thus corrupting `errno`.

systemd-user: move pam snippet default location to /usr/lib/pam.d

bootctl: don't accidentally propagate errors in "bootctl status"

Fixes: #16989

test-network: update tests for issue #16784

network: replace FRA_IFNAME -> FRA_IIFNAME

No functional change, as FRA_IFNAME is an alias of FRA_IIFNAME.

network: also process RTM_NEWRULE or RTM_DELRULE message which does not contain src and dst addresses

Fixes #16784.

network: fix the default mask for FirewallMark=

And always send FRA_FWMASK if FirewallMark= is set.

C.f. https://github.com/torvalds/linux/commit/b8964ed9fa727109c9084abc807652ebfb681c18

Partially fixes #16784.

network: do not assign return value if the parse_fwmark_fwmask() fails

This also removes redundant logs, and makes input string not copied if
it does not contain '/'.

network: add debug log for removing routing policy rules

network: also logs priority of routing policy rules

network: update log message for rtnl messages

core: downgrade error level and ignore several non-critical errors

homed: downgrade log level

journal: downgrade log level

login: downgrade log level if the error will be ignored

nspawn: downgrade log level if the error will be ignored

repart: downgrade log level

resolve: downgrade error level when the error will be ignored

resolve: check DNSSD service name template before assigning it

conf-parser: logs about OOM error

conf-parser: use SYNTHETIC_ERRNO() at one more place

timesync: downgrade error level when the error will be ignored

xdg-autostart-generator: downgrade error level when the error will be ignored

vlan: downgrade error level if the error will be ignored

ethtool: downgrade log level when the error will be ignored

network: slightly update log message

network do not ignore OOM error in config_parse_macsec_key_id()

network: use _cleanup_ attribute at one more place

network: do not ignore OOM error in wireguard_decode_key_and_warn()

network: unify config_parse_wireguard_public_key() and config_parse_wireguard_preshared_key()

udev: fix indentation

Follow-up for 90e30d767a32dc93c9ee94ad8b8d665eecf79e96.

Merge pull request #17000 from poettering/network-fixlets

network: a bunch of unimportant cleanups across the board

Minor simplification in sockaddr_un_set_path()

Use sockaddr_un_set_path() in socket_address_parse()

Two functional changes:
- "/" is now refused. The test is adjusted.
- The trailing NUL is *not* included in the returned size for abstract size. The
  comments in sockaddr_un_set_path() indicate that this is the right thing to do,
  and the code in socket_address_parse() wasn't doing that.

shared/socket-netlink: set output in socket_address_parse_netlink() only on success

Allow interface scopes to be specified in ListenStream=

Closes #12624.

The formatting in systemd.socket.xml is updated a bit.

Currently in_addr_port_ifindex_name_to_string() always prints the ifindex
numerically. This is not super useful since the interface numbers are
semi-random. Should we use interface names in preference?

resolved: drop duplicated check

The same conditional appears a few lines down.

shared: don't unconditionally set SOCK_STREAM as type in socket_address_parse()

We would set .type to a fake value. All real callers (outside of tests)
immediately overwrite .type with a proper value after calling
socket_address_parse(). So let's not set it and adjust the few places
that relied on it being set to the fake value.

socket_address_parse() is modernized to only set the output argument on
success.

shared: make socket_address_parse use the generic parser for IPv[46] addresses

One special syntax is not supported anymore: "iface:port" would be parsed as an
interface name plus numerical port, equivalent to "[::]%iface:port". This was
added in 542563babda, but was undocumented, and we had no tests for it. It seems
that this actually wasn't doing anything useful, because the kernel only uses the
scope identifier for link-local addresses.

Get rid of in_addr_port_from_string_auto() again

With the commit "shared/socket-netlink: only allow ifindex if explicitly supported"
this helper is not necessary anymore.

shared: check interface name validity in in_addr_port_ifindex_name_from_string_auto()

We don't try to resolve invalid ifnames as all. A different return
code is used. This difference will be verified later in test_socket_address_parse()
when socket_address_parse() is converted to use
in_addr_port_ifindex_name_from_string_auto().

basic: show interface scope in sockaddr_pretty()

If the interface scope is specified, this changes the meaning of the address
quite significantly. Let's show the IPv6 scope_id if present.

Sadly we don't even have a test for sockaddr_pretty() output :(
This will be implicitly tested through socket_address_parse() later on.

basic: convert ifname_valid_full() to take flags and allow numeric interfaces

shared/socket-netlink: only allow ifindex if explicitly supported

Instead of ignoring ifindex if not wanted, refuse it is the caller
does not allow it.

test-socket-netlink: print the proper expected string

We would use the return value from the tested function to decide
what to print as "expected", which is confusing when something is wrong
with the tested function.

test-in-addr-util: add log headers

resolved: unify the two functions to create main stubs

There is a small functional difference: IP_TTL==1 is now also set for the UDP
socket. I assume that it wasn't set by mistake.

resolved: unify the two functions to create extra stubs

There is a minor functional change:
IPV6_FREEBIND is set of IPv6 sockets, not IP_FREEBIND. This was missed in
af8b1384, but I noticed only after the merging the two functions.

And a not-so-minor functional chagnge:
7216a3b5dcde36245 changed manager_dns_stub_tcp_fd_extra() to return the fd even
if the source was already initialized, but it didn't do the same change for
manager_dns_stub_udp_fd_extra(), so it would return 0 in that case. But
0354029bf572489b uses manager_dns_stub_udp_fd_extra() when preparing to call
manager_send(), and will pass 0 as the fd in that case. For both socket types
fd is now always returned.

resolve: move handler functions higher

No functional change, preparation for subsequent refactoring.

ethtool-util: don't pass error value that isn't used to log_syntax

network: don't fail on various config parse errors

We typically don't fail on config parse errors (to maximize compat),
let's not do this in these cases either.

tree-wide: prefer AF_xyz over PF_xyz

This really doesn't matter given that AF_xyz and PF_xyz are equivalent
in all ways, but we almost always use AF_xyz, hence stick to it
universally and convert the remaining PF_ to AF_

man: document that sd_bus_message_close_container() may only be called at end of container

Prompted-by: https://lists.freedesktop.org/archives/systemd-devel/2020-September/045264.html

Merge pull request #16985 from poettering/resolve-dns-stub-extra-fixlets

resolved: dns stub listener extra fixes

Merge pull request #16997 from poettering/foreach-word-followup

fix two coverity issues

cryptsetup: Fix null pointer dereference (#16987)

cryptsetup: Fix null pointer dereference

Fix null pointer dereference in the pkcs11 related code of systemd-cryptsetup

sd-login: fix memory leak

Fixes: CID1432652

socket: fix copy/paste error

Fixes: CID1432653

logind: fix merge issue

The two PRs #16664 and #16635 individually passed CI, but when combined
cannot build. Since both are merged now, let's fix that.

login: Add KEY_RESTART handling

KEY_RESTART is widely used in Linux to indicate device reboot.
So lets handle it in the same fashion as KEY_POWER.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>

logind.conf: document UserStopDelaySec in logind.conf

Merge pull request #16635 from keszybz/do-not-for-each-word

Drop FOREACH_WORD

Merge pull request #16972 from wusto/ambient-and-keep-caps-corrections

Ambient capabilities documenation and keep-caps usage corrections

Merge pull request #16984 from yuwata/make-log_xxx_error-void

Make log_xxx_error() or friends return void

Merge pull request #16982 from yuwata/socket-buffer-size

Fixes for socket buffer size

core: fix set keep caps for ambient capabilities

The securebit keep-caps retains the capabilities in the permitted set
over an UID change (ambient capabilities are cleared though).

Setting the keep-caps securebit after the uid change and before execve
doesn't make sense as it is cleared during execve and there is no
additional user ID change after this point.

Altough the documentation (man 7 capabilities) is ambigious, keep-caps
is reset during execve although keep-caps-locked is set. After execve
only keep-caps-locked is set and keep-caps is cleared.

core: fix comments on ambient capabilities

The comments on the code for ambient capabilities was wrong/outdated.

Rename strv_split_extract() to strv_split_full()

Now that _full() is gone, we can rename _extract() to have the usual suffix
we use for the more featureful version.

Remove FOREACH_WORD and friends

tree-wide: replace strv_split_full() with strv_split_extract() everywhere

Behaviour is not identical, as shown by the tests in test-strv.
The combination of EXTRACT_UNQUOTE without EXTRACT_RELAX only appears in
the test, so it doesn't seem particularly important. OTOH, the difference
in handling of squished parameters could make a difference. New behaviour
is what both bash and python do, so I think we can ignore this corner case.

This change has the following advantages:
- the duplication of code paths that do a very similar thing is removed
- extract_one_word() / strv_split_extract() return a proper error code.

test-string-util: stop testing FOREACH_WORD

shared/fstab-util: replace FOREACH_WORD_SEPARATOR() with open-coded loop

The tricky part here is that the function is not allowed to fail in this code
path. Initially, I wanted to change the return value to allow it to fail, but
this cascades through all the places where fstab_test_option() and friends are
used; updating all those sites would be a lot of work. And since quoting is not
allowed here, a simple loop with strcspn() is easy to do.

shared/fstab-util: use free_and_str[n]dup()

No functional change. I'm keeping this separate to make review easier.

Use extract_first_word() in generated conf parsers

nspawn: use extract_first_word()

getty-generator: use extract_first_word()

sd-journal: use extract_first_word()

delta: use extract_first_word()

cryptsetup: use extract_first_word()

core/load-fragment: use extract_first_word()

This is much nicer, and also fixes a potential overflow when we used
'word' in log_error() as if it was a NUL-terminated string.

Let sd_machine_get_ifindices() omit the output param too

Nowadays we do that almost everywhere, let's also do it here.

Rewrite sd_machine_get_ifindices() to avoid FOREACH_WORD()

If we fail to parse the index, the failure is propogated as -EUNCLEAN.
(-EINVAL would be confused with invalid args to the function itself.)

Fix output value of sd_seat_get_sessions() and drop FOREACH_WORD use

sd_seat_get_sessions() would return 0 in the 'n_uids' (now 'ret_n_uids') output
parameter when 'uid' (now 'ret_uids') was passed as NULL.

While at it, drop FOREACH_WORD() use.

Also use any whitespace as separator. In practice this shouldn't matter, since
logind always uses spaces, but it seems nicer to not specify this explicitly,
and the default is more flexible.

sd-device: use extract_first_word()

basic/cgroup-util: port over to string_contains_word()

logind: use extract_first_word()

udev: warn if failed to set buffer size for device monitor

network: increase receive buffer size for device monitor

If networkd creates huge amount of netdevs, then the buffer of device
monitor becomes easily flowed.

Hopefully fixes #16865.

network: do not start device monitor if /sys is read-only

Follow-up for bf331d87171b7750d1c72ab0b140a240c0cf32c3.

network: honor the buffer size specified in networkd.socket

core/socket: use fd_set_{rcv,snd}buf()

sd-device-monitor: use fd_set_rcvbuf()

util: introduce fd_set_{snd,rcv}buf()

util: try to set with SO_{RCV,SND}BUFFORCE when requested size is larger than the kernel limit

The commit 10ce2e0681ac16e7bb3619b7bb1a72a6f98a2f2c inverts the order of
SO_{RCV,SND}BUFFORCE and SO_{RCV,SND}BUF. However, setting buffer size with
SO_{RCV,SND}BUF does not fail even if the requested size is larger than
the kernel limit. Hence, SO_{RCV,SND}BUFFORCE will not use anymore and
the buffer size is always limited by the kernel limit even if we have
the priviledge to ignore the limit.

This makes the buffer size is checked after configuring it with
SO_{RCV,SND}BUF, and if it is still not sufficient, then try to set it
with FORCE command. With this commit, if we have enough priviledge, the
requested buffer size is correctly set.

Hopefully fixes #14417.

util: refuse to set too large value for socket buffer size

network: ignore error on increasing netlink receive buffer size

Merge pull request #16973 from poettering/btrfs-dev-root

print friendly error message if btrfs tells us /dev/root was backing block device of root fs