Page MenuHomeFreeBSD

syslogd: Open forwarding socket descriptors
ClosedPublic

Authored by jfree on Oct 14 2024, 1:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 2, 9:08 AM
Unknown Object (File)
Sat, Nov 30, 12:38 PM
Unknown Object (File)
Thu, Nov 28, 12:50 AM
Unknown Object (File)
Tue, Nov 26, 5:27 PM
Unknown Object (File)
Tue, Nov 26, 2:48 AM
Unknown Object (File)
Tue, Nov 26, 2:47 AM
Unknown Object (File)
Thu, Nov 21, 12:10 AM
Unknown Object (File)
Sat, Nov 16, 9:42 AM
Subscribers

Details

Summary

Previously, when forwarding a message to a remote address, the target's
addrinfo was saved at config-parse-time. When message-deliver-time came,
the message's addrinfo was passed into sendmsg(2) and delivered by the
first available inet socket.

The use of sendmsg(2) is prohibited in Capsicum capability mode, so
sockets are now opened and connected to their remote peers at
config-parse-time when executing outside of the capability sandbox.

These sockets are then used with send(2), allowing forwarding to be
performed inside of the capability sandbox.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jfree requested review of this revision.Oct 14 2024, 1:56 AM

This looks good to me, thanks. A couple of minor nits.

usr.sbin/syslogd/syslogd.c
367–372

Hopefully f->f_addr_fds == NULL implies f->f_num_addr_fds == 0, in which case there's no need to check this condition explicitly.

3043

It'd be nicer to use calloc() here IMO.

This revision is now accepted and ready to land.Oct 14 2024, 12:45 PM
jfree marked 2 inline comments as done.

Address Mark's comments: use calloc() instead of malloc() for
memory allocations. This is done to avoid potential integer overflow
in total allocation size.

This revision now requires review to proceed.Oct 19 2024, 6:29 PM
usr.sbin/syslogd/syslogd.c
367–372

Yes. f->f_addr_fds == NULL does imply f->f_num_addr_fds == 0

markj added inline comments.
usr.sbin/syslogd/syslogd_cap_config.c
218

Same here, it'd be nice to use calloc().

This revision is now accepted and ready to land.Oct 21 2024, 2:27 PM
jfree marked an inline comment as done.
  • Use calloc(3) instead of malloc(3) for array allocations.
  • Add missing error checking for a memory allocation
This revision now requires review to proceed.Sun, Nov 17, 7:57 PM
usr.sbin/syslogd/syslogd.c
1751

I'd suggest writing sockfd = f->f_addr_fds[0] here.

1770

The new getsockopt() and getpeername() calls are only needed to implement dprintf(), but that's a no-op unless debug printing is enabled. IMO it would be better to avoid this extra work unless Debug is true.

2666
3048
usr.sbin/syslogd/syslogd_cap_config.c
223

Not new with this diff, so can be fixed separately, but: can we use nvlist_take_descriptor_array() instead of duping? Same for the F_FILE case.

jfree marked 4 inline comments as done.
  • Minor style changes
  • Only call getsockopt() and getpeername() when Debug is non-zero
usr.sbin/syslogd/syslogd_cap_config.c
223

Not new with this diff, so can be fixed separately, but: can we use nvlist_take_descriptor_array() instead of duping? Same for the F_FILE case.

We could. I addressed this in response to your other comment about it.

I use nvlist_get_descriptor_array() here because nvlist_to_filed() and filed_to_nvlist() do not consume their arguments. i.e. the nvlist passed into nvlist_to_filed() is marked const. Using nvlist_take_descriptor_array() would modify that nvlist.

usr.sbin/syslogd/syslogd.c
1806

Sorry I didn't catch this before - I believe you can still use sendmsg() (so as to avoid having to loop over the iovec), so long as msghdr.msg_name = NULL. That would be cleaner IMO.

usr.sbin/syslogd/syslogd.c
1806

Sorry I didn't catch this before - I believe you can still use sendmsg() (so as to avoid having to loop over the iovec), so long as msghdr.msg_name = NULL. That would be cleaner IMO.

Ah! I could've sworn I checked syscalls.master and didn't see CAPENABLED on sendmsg() before doing this. I guess I didn't. I will definitely use sendmsg() then.

jfree marked an inline comment as done.

Use sendmsg(2) to send iovec list.

This revision is now accepted and ready to land.Sun, Nov 24, 6:40 PM
This revision was automatically updated to reflect the committed changes.