Page MenuHomeFreeBSD

Add RFC 5424 syslog message output to syslogd.
ClosedPublic

Authored by ed on Apr 8 2018, 7:40 AM.

Details

Summary
  • Move all of the code responsible for transmitting log messages into a separate function, fprintlog_write().
  • Instead of manually modifying a list of iovecs, add a structure iovlist with some helper functions.
  • Alter the F_FORW (UDP message forwarding) case to also use iovecs like the other cases. Use sendmsg() instead of sendto().
  • In the case of F_FORW, truncate the message to a size dependent on the address family (AF_INET, AF_INET6), as proposed by RFC 5426.
  • Move all traditional message formatting into fprintlog_bsd(). Get rid of some of the string copying and snprintf()'ing. Simply emit more iovecs to get the job done.
  • Increase ttymsg()'s limit of 7 iovecs to 32. Add a definition for this limit, so it can be reused by iovlist.
  • Add fprintlog_rfc5424() to emit RFC 5424 formatted log entries.
  • Add a "-5" command line option to enable RFC 5424 formatting. It would have been nicer if we supported "-o rfc5424", just like on NetBSD. Unfortunately, the "-o" flag is already used for a different purpose on FreeBSD.
  • Don't truncate hostnames in the RFC 5424 case, as suggested by that specific RFC.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ed created this revision.Apr 8 2018, 7:40 AM
ed updated this revision to Diff 41265.Apr 8 2018, 2:23 PM

Always show the <PRI>VERSION. Don't let it depend on LogFacPri.

It looks like rsyslog and friends also do this when enabling RFC 5424
output.

ed updated this revision to Diff 41267.Apr 8 2018, 2:32 PM

Don't conditionalize the addition of the pointer.

kib added inline comments.Apr 8 2018, 3:21 PM
usr.sbin/syslogd/syslogd.c
1592 ↗(On Diff #41267)

Are you sure that such assert is the good way to handle the situation ? You truncate the output anyway, at least for some formats. IMO it is somewhat frightening to know that such important service as syslogd could be shut down by some inaccuracy instead of tolerating it.

In other words, I believe that in case of programming error, most people would prefer to get even truncated message instead of losing all their logs.

If you make iov dynamically allocated, you re-invent struct uio.

ed updated this revision to Diff 41269.Apr 8 2018, 4:04 PM

Process feedback from kib@.

ed added inline comments.Apr 8 2018, 4:06 PM
usr.sbin/syslogd/syslogd.c
1592 ↗(On Diff #41267)

That's a good point. I've changed the code to tolerate running out of iovecs.

Indeed; this is sort of a poor man's uio. It looks like we have nothing like that in userspace yet. :-/

Hi Ed, I haven’t had a chance to do a full review yet, but here are some initial comments for your consideration.

Also, would the architecture of the new message creation functions change if we have options to vary the transport in the future (e.g. UDP, TCP plaintext, TLS)?

usr.sbin/syslogd/syslogd.8
63 ↗(On Diff #41269)

Rather than a binary -5 option, I think we should use a generic output modifier, which would be coupled with a string argument, to allow more than just 2 output options (as per NetBSD). Whilst we can’t use -o, another character could be used.

In the future, there may be new output formats (e.g. RFC5424 version 2).

We may also want to set the transport (e.g. UDP, TCP plaintext, TLS). This would probably be a separate flag, but could be combined.

E.g.
-o syslog_rfc5425
-o syslog_rfc5426

usr.sbin/syslogd/syslogd.c
1822 ↗(On Diff #41269)

Please use fprintlog_rfc3164 instead of fprintlog_bsd to be consistent with the input parsing code.

kib accepted this revision.Apr 9 2018, 9:01 AM
This revision is now accepted and ready to land.Apr 9 2018, 9:01 AM
ed added a comment.Apr 9 2018, 8:08 PM

Hi Ben,

Also, would the architecture of the new message creation functions change if we have options to vary the transport in the future (e.g. UDP, TCP plaintext, TLS)?

Adding additional transport protocols should become somewhat easier because of this change. I think all of that stuff can now be placed in fprintlog_write() without modifying any of the message creation in fprintlog_rfc*().

usr.sbin/syslogd/syslogd.8
63 ↗(On Diff #41269)

Configuring UDP vs TCP vs TLS is something that likely needs to be done in syslog.conf as opposed to using command line flags. Still, I've renamed the flag to -O, having values similar to NetBSD. Does that look all right?

usr.sbin/syslogd/syslogd.c
1822 ↗(On Diff #41269)

Done!

ed updated this revision to Diff 41301.Apr 9 2018, 8:08 PM

Process woodsb02's feedback.

This revision now requires review to proceed.Apr 9 2018, 8:08 PM
ed added a comment.Apr 12 2018, 9:22 AM

Ben: any final thoughts before I commit this?

This revision was not accepted when it landed; it landed in state Needs Review.Apr 15 2018, 8:34 AM
This revision was automatically updated to reflect the committed changes.