Add RPC for injecting messages in the system log by troglobit · Pull Request #1643 · kernelkit/infix

troglobit · 2026-09-15T15:12:19Z

Description

Test systems and scripts have had no clean way to leave a mark in a device's log, short of SSH and logger(1). This adds an RPC that hands a message to the local syslog daemon, exposing the full RFC 5424 header: severity, facility, app-name, msgid, and structured data. Defaults follow logger, user.notice, with app-name defaulting to the calling user. Messages are time stamped on arrival and follow the configured /syslog filtering and forwarding rules, so the device decides where they end up, not the caller.

confd: new infix-syslog revision with the log RPC, handler built
on syslogp_r() from sysklogd's libsyslog, docs and ChangeLog
cli: admin-exec log [severity S] [facility F] [msgid M] MESSAGE.
The rpc tool now sets the session originator to the calling user and
only splits comma-separated values for leaf-lists, a message with a comma
previously failed
infamy: target.log() helper, RESTCONF call_dict() implemented (it
used to return without sending), four tests converted from SSH + logger
test: new syslog/rpc_log case verifying header fields and defaults

Checklist

Tick relevant boxes, this PR is-a or has-a:

Add an infix-syslog:log RPC that hands a message to the local syslog daemon. The full RFC 5424 header is exposed: severity, facility, app-name, msgid, and structured data. Defaults follow logger(1), user.notice, with app-name defaulting to the name of the calling user. The message is time stamped on arrival and goes through the regular /syslog filtering and forwarding rules, so where it ends up is decided by the device configuration, not the caller. The handler uses syslogp_r() from the sysklogd libsyslog, which passes msgid and structured data through and sets the tag per call without touching confd's own openlog() identity. The callback's event session runs as confd, so the calling user is taken from the event originator: netopeer2 pushes it as originator data, the CLI sets it as originator name. RESTCONF sets neither, there the originator name is used as-is. Fixes #1639 Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Front-end to the infix-syslog:log RPC. Optional severity, facility, and msgid keywords precede the message, which is the rest of the line: admin@example:/> log severity warning msgid test-start Test 42 starting Two fixes to the rpc tool the command is built on. It now sets the session originator name to the calling user, like klish-plugin-sysrepo, so the RPC can default app-name to the user. And it only splits comma-separated values for leaf-lists, previously any leaf value with a comma became two instances and the RPC failed: $ rpc /infix-syslog:log message "Kilroy was here, again" Issue #1639 Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Wrap the infix-syslog:log RPC so tests can leave markers in a DUT's system log over the management API instead of SSH and logger(1). Facility names are module-qualified in the helper, and structured data is given as a dict of dicts to keep call sites short. The RESTCONF backend's call_dict() only tracked coverage and returned without sending anything, so RPCs with input silently did nothing over RESTCONF. POST to /restconf/operations/MODULE:NAME with the input wrapped per RFC 8040, like call_action() does. Convert the tests that logged via SSH. The remote and hostname_filter tests keep using logger(1), they deliberately exercise the remote client path. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

Log to an RFC 5424 formatted file and check app-name, msgid, and structured data land in the header, and that a bare message gets the default app-name with empty msgid and structured data. Signed-off-by: Joachim Wiberg <troglobit@gmail.com>

mattiaswal

Security review of the new log RPC. Verified against libyang (yanglint) with the exact type definitions from this PR, and against the sysklogd 2.7.2 sources Infix ships. Details inline; summary ranked by severity:

High: format string injection via msgid. libsyslog's vsyslogp_r() concatenates msgid into the printf format it hands to vsnprintf(), and the pattern [!-~]+ allows %. Runs inside confd as root.
Medium: log line forgery via structured-data/param/value. Control characters are not escaped, syslogd does not sanitize SD (only MSG), and fmt5424 writes it verbatim to RFC 5424 files and remote forwarding.
Medium: unbounded SD is truncated in libsyslog's 2048-byte buffer, after which syslogd's RFC 5424 parser drops the whole message silently.
Medium (design): any user with exec rights (factory NACM: exec-default permit) can forge entries tagged as any daemon, in auth/authpriv, or at emergency severity (walled to all ttys and /dev/console). Only kern is neutralized by syslogd.
Low: no rate limiting; libsyslog's sendto() is blocking and confd's RPC callback is SR_SUBSCR_NO_THREAD.
Low: default app-name from sr_session_get_orig_name() is client-controlled.

What I checked and found fine: the sd-name typedef matches RFC 5424 exactly (rejects =, ], ", SP); sd_build() length arithmetic is correct and sysrepo validates mandatory/key leaves before the callback; ASCII control characters in message are rejected by libyang (C0 except TAB/LF/CR) or neutralized by syslogd (LF to space, others to ^X); the klish script quoting and the rpc/copy changes are sound.

Note that -8 on syslogd disables C1 masking, so UTF-8 encoded C1 controls (U+0085, U+009B), U+2028 and bidi overrides in message reach log files and show log untouched. Low severity, mentioned for completeness.

mattiaswal · 2026-09-16T12:33:01Z

+      leaf msgid {
+        type string {
+          length "1..32";
+          pattern '[!-~]+';


High: format string injection. This pattern permits %, and sysklogd's vsyslogp_r() builds its printf format by concatenating msgid into it (sysklogd 2.7.2 src/syslog.c L451-452, then vsnprintf(p, tbuf_left, fmt_cpy, ap) at L507). A caller can therefore pass msgid = %s%s%s%s%s%s to crash confd, or %n... for a write primitive, and this runs as root. yanglint confirms %n%n%n%n is accepted by this type.

Suggest excluding % here, e.g. pattern '[!-$&-~]+';, and also escaping % as %% in rpc_log() so the C side does not depend on the model. app-name is not affected (libsyslog uses snprintf("%s ", tag)), and sd/message are passed as %s arguments.

Awesome feedback, will look into this asap!

mattiaswal · 2026-09-16T12:33:01Z

+	log.log_tag = tag;
+	sd = sd_build(in);
+	if (sd)
+		syslogp_r(pri, &log, msgid, "%s", "%s", sd, msg);


See the comment on the msgid leaf: msgid is used as part of the format string inside syslogp_r(), not as a plain argument. Even with the YANG pattern tightened, I would escape % to %% here (or reject the value) before passing it on. The "%s" handling of sd and msg is correct.

Will fix, thanks!

mattiaswal · 2026-09-16T12:33:01Z

+          }
+
+          leaf value {
+            type string;


Medium: log line forgery and silent message loss.

value is an unrestricted string, and sd_escape() only escapes ", \ and ]. libyang accepts LF, CR, DEL and C1 controls here. syslogd's parsemsg_rfc5424() accepts any bytes inside PARAM-VALUE except an unescaped ", keeps the SD block raw (buffer.sd = msg), and fmt5424() writes it verbatim. Only MSG goes through parsemsg_remove_unsafe_characters(). So a value containing \n produces a fabricated extra log line in every log-format rfc5424 file and in RFC 5424 remote forwarding.

There is no bound on value length or list size, but libsyslog formats the whole packet into tbuf[2048]. A long SD is truncated mid-element, syslogd's FAIL_IF("STRUCTURED-NAME", ...) then drops the entire message, and the caller gets SR_ERR_OK.

Suggest something like length "1..255" plus a pattern excluding control characters, and escaping or stripping controls in sd_escape() as a belt-and-braces measure.

I'll take a deeper look at this too, the formatting rules are strict but allows quite a lot still, so this needs some careful thinking. Might even spill over as fixes in the sysklogd project, so thank you! 🙏

mattiaswal · 2026-09-16T12:33:01Z

+}
+
+/* RFC 5424 PARAM-VALUE: escape '"', '\\', and ']' */
+static char *sd_escape(char *ptr, const char *value)


Consider also handling control characters here (e.g. replace anything < 0x20 and 0x7f with a space), since syslogd sanitizes MSG but not SD. See the comment on the value leaf for the forgery scenario.

mattiaswal · 2026-09-16T12:33:01Z

+    reference "RFC 5424: The Syslog Protocol";
+
+    input {
+      leaf message {


FYI on control characters in message, since it came up: libyang enforces RFC 7950 string rules, so C0 controls other than TAB/LF/CR, surrogates and noncharacters are rejected at parse time (ESC cannot get in). LF/CR/DEL do get through, but syslogd's parsemsg_remove_unsafe_characters() turns LF into a space and the rest into ^X, so this leaf cannot forge a second log line. Because Infix runs syslogd -8, C1 masking is off, so UTF-8 encoded U+0085/U+009B, U+2028 and bidi overrides such as U+202E are logged as-is. Low severity; a pattern like [^\p{Cc}\p{Cf}\p{Zl}\p{Zp}]+ would close it if wanted.

One more thing: length "1..2048" counts characters, while libsyslog's buffer is 2048 bytes for the entire packet including header and SD, so long messages are silently truncated. Worth mentioning in the description or lowering the limit.

mattiaswal · 2026-09-16T12:33:01Z

+   * RPCs
+   */
+
+  rpc log {


Medium (design): forgery and authorization. Callers pick app-name, facility and severity freely. That allows entries in /var/log/auth.log tagged sshd with facility auth, or emergency messages that syslogd walls to every logged-in tty and to /dev/console (factory *.=emerg * and console.conf). Only kern is neutralized, since syslogd remaps LOG_KERN from /dev/log unless started with -k.

The factory NACM has exec-default permit, so operators and any non-guest user can call this. Options: a factory NACM rule limiting infix-syslog:log exec to admin (or an explicit operator permit), refusing auth/authpriv, and/or always adding a fixed marker (e.g. an SD element or msgid prefix) so injected messages stay distinguishable from genuine ones in forwarded logs. At minimum the docs should say that the header fields are caller-supplied and not authenticated.

I was considering marking the RPC with nacm:default-deny-all;, so this is great feedback, thanks! It's the age-old question of flexibility vs security.

Guess it can be good, we should really be more strict about access in general and use nacm:default-deny-all; more.

mattiaswal · 2026-09-16T12:33:01Z

+	return sd;
+}
+
+static int rpc_log(sr_session_ctx_t *session, uint32_t sub_id, const char *op_path,


Low: no rate limiting. Each call can write up to 2048 bytes to a typically RAM-backed /var/log, rotating away real evidence and flooding remote collectors. Also, libsyslog's sendto() on the connected AF_UNIX datagram socket is blocking (no SOCK_NONBLOCK), and this callback is registered with SR_SUBSCR_NO_THREAD, so a full syslogd receive queue would stall the whole confd event loop. A simple token bucket here would cover both.

mattiaswal · 2026-09-16T12:33:01Z

+ * the originator: netopeer2 pushes [nc-sid, username], the CLI and the
+ * rpc tool set their originator name to the user.
+ */
+static const char *log_user(sr_session_ctx_t *session)


Low: sr_session_get_orig_name() is set by the client itself (copy.c does this too), so any sysrepo client can claim any name, and one claiming netopeer2 without orig data falls through to the literal string netopeer2 as tag. No privilege is gained since app-name is caller-controlled anyway, but the docs should not present the default as an authenticated identity.

mattiaswal

Duplicate of the review above, posted twice by mistake. Comments removed.

mattiaswal · 2026-09-17T06:23:02Z

Didn't find anything when reviewing but i was scared about something like #1643 (comment) so i let claude get a round of this. This i feel is crutial, other it looks fine.

troglobit · 2026-09-17T07:10:47Z

Didn't find anything when reviewing but i was scared about something like #1643 (comment) so i let claude get a round of this. This i feel is crutial, other it looks fine.

Agreed, will have a look at it over the weekend, thanks! 🙇‍♂️

troglobit added 4 commits September 15, 2026 16:51

troglobit marked this pull request as draft September 15, 2026 15:27

troglobit changed the title ~~Rpc log~~ Add infix-syslog:log RPC for injecting messages in the system log Sep 16, 2026

troglobit requested a review from mattiaswal September 16, 2026 08:43

troglobit changed the title ~~Add infix-syslog:log RPC for injecting messages in the system log~~ Add RPC for injecting messages in the system log Sep 16, 2026

troglobit marked this pull request as ready for review September 16, 2026 08:43

troglobit linked an issue Sep 16, 2026 that may be closed by this pull request

Add RPC for injecting log messages to a device #1639

Open

mattiaswal reviewed Sep 16, 2026

View reviewed changes

Conversation

troglobit commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

mattiaswal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattiaswal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattiaswal commented Sep 17, 2026

Uh oh!

troglobit commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

troglobit commented Sep 15, 2026 •

edited

Loading

mattiaswal left a comment •

edited

Loading