Attachment 3641 Details for Bug 3478 – another version, logging via monitor

[patch] another version, logging via monitor

sandbox-debug2.diff (text/plain), 5.38 KB, created by Damien Miller on 2022-12-12 18:02:38 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2022-12-12 18:02:38 AEDT

Size: 5.38 KB

patch

obsolete

>diff --git a/monitor_wrap.c b/monitor_wrap.c
>index b2c85205e..05a1149ed 100644
>--- a/monitor_wrap.c
>+++ b/monitor_wrap.c
>@@ -80,33 +80,62 @@
> extern struct monitor *pmonitor;
> extern struct sshbuf *loginmsg;
> extern ServerOptions options;
>+static sig_atomic_t in_logsend;
> 
>+/*
>+ * Just like atomicio(vwrite, ...) but simpler.
>+ * We can't use atomicio() in signal handlers as it calls poll(3) in the
>+ * fallback path, and the replacement implementation in libopenbsd-compat
>+ * calls malloc(3), which isn't signal handler safe.
>+ */
>+static int
>+awrite(int fd, const void *m, size_t len)
>+{
>+	ssize_t r;
>+	size_t o;
>+	const u_char *buf = (const u_char *)m;
>+
>+	for (o = 0; o < len;) {
>+		r = write(fd, buf + o, len - o);
>+		if (r <= 0) {
>+			if (r == EINTR || r == EAGAIN || r == EWOULDBLOCK)
>+				continue;
>+			return -1;
>+		}
>+		o += r;
>+	}
>+	return 0;
>+}
>+
>+/*
>+ * NB. portable OpenSSH uses a signal handler-safe version of this function
>+ * as we need to be able to log sandbox violations that we receive in
>+ * signal handler context.
>+ */
> void
> mm_log_handler(LogLevel level, int forced, const char *msg, void *ctx)
> {
>-	struct sshbuf *log_msg;
>+	u_char hdr[4 + 4 + 4 + 4];
> 	struct monitor *mon = (struct monitor *)ctx;
>-	int r;
> 	size_t len;
> 
>-	if (mon->m_log_sendfd == -1)
>-		fatal_f("no log channel");
>+	len = strlen(msg);
> 
>-	if ((log_msg = sshbuf_new()) == NULL)
>-		fatal_f("sshbuf_new failed");
>+	/* avoid reentrance */
>+	if (in_logsend || len > INT_MAX)
>+		_exit(0); /* can't recover */
> 
>-	if ((r = sshbuf_put_u32(log_msg, 0)) != 0 || /* length; filled below */
>-	    (r = sshbuf_put_u32(log_msg, level)) != 0 ||
>-	    (r = sshbuf_put_u32(log_msg, forced)) != 0 ||
>-	    (r = sshbuf_put_cstring(log_msg, msg)) != 0)
>-		fatal_fr(r, "assemble");
>-	if ((len = sshbuf_len(log_msg)) < 4 || len > 0xffffffff)
>-		fatal_f("bad length %zu", len);
>-	POKE_U32(sshbuf_mutable_ptr(log_msg), len - 4);
>-	if (atomicio(vwrite, mon->m_log_sendfd,
>-	    sshbuf_mutable_ptr(log_msg), len) != len)
>-		fatal_f("write: %s", strerror(errno));
>-	sshbuf_free(log_msg);
>+	/* u32 len, u32 level, u32 forced, string msg */
>+	POKE_U32(hdr, 4 + 4 + 4 + len);
>+	POKE_U32(hdr + 4, level);
>+	POKE_U32(hdr + 8, forced);
>+	POKE_U32(hdr + 12, len);
>+
>+	in_logsend = 1;
>+	if (awrite(mon->m_log_sendfd, hdr, sizeof(hdr)) != 0 ||
>+	    awrite(mon->m_log_sendfd, msg, len) != 0)
>+		_exit(0); /* can't recover */
>+	in_logsend = 0;
> }
> 
> int
>diff --git a/sandbox-seccomp-filter.c b/sandbox-seccomp-filter.c
>index cec43c463..e68d7975c 100644
>--- a/sandbox-seccomp-filter.c
>+++ b/sandbox-seccomp-filter.c
>@@ -15,15 +15,15 @@
>  */
> 
> /*
>- * Uncomment the SANDBOX_SECCOMP_FILTER_DEBUG macro below to help diagnose
>- * filter breakage during development. *Do not* use this in production,
>- * as it relies on making library calls that are unsafe in signal context.
>+ * Uncomment the SANDBOX_SECCOMP_FILTER_DEBUG macro below and run sshd with
>+ * stderr attached (sshd -De ... or sshd -d ...) to receive notifications of
>+ * sandbox violations to stderr. E.g.
>  *
>- * Instead, live systems the auditctl(8) may be used to monitor failures.
>- * E.g.
>+ * Alternately, live systems the auditctl(8) may be used to monitor
>+ * failures. E.g.
>  *   auditctl -a task,always -F uid=<privsep uid>
>  */
>-/* #define SANDBOX_SECCOMP_FILTER_DEBUG 1 */
>+#define SANDBOX_SECCOMP_FILTER_DEBUG 1
> 
> #if 0
> /*
>@@ -79,6 +79,13 @@
> #ifdef SANDBOX_SECCOMP_FILTER_DEBUG
> # undef SECCOMP_FILTER_FAIL
> # define SECCOMP_FILTER_FAIL SECCOMP_RET_TRAP
>+# ifdef WITH_OPENSSL
>+#  include <openssl/dh.h>
>+# endif /* WITH_OPENSSL */
>+# ifdef GSSAPI
>+#  include "ssh-gss.h"
>+# endif /* GSSAPI */
>+# include "monitor_wrap.h"
> #endif /* SANDBOX_SECCOMP_FILTER_DEBUG */
> 
> #if __BYTE_ORDER == __LITTLE_ENDIAN
>@@ -364,17 +371,42 @@ ssh_sandbox_init(struct monitor *monitor)
> }
> 
> #ifdef SANDBOX_SECCOMP_FILTER_DEBUG
>-extern struct monitor *pmonitor;
>-void mm_log_handler(LogLevel level, int forced, const char *msg, void *ctx);
>+/* convert an integer to a hex string; for use in signal handler */
>+static const char *
>+ntoh(long unsigned int n)
>+{
>+	static char ret[sizeof(long unsigned int) * 2 + 2 + 1];
>+	int i = sizeof(ret) - 2;
>+
>+	if (n == 0)
>+		return "0";
>+	while (n > 0) {
>+		ret[i--] = "0123456789abcdef"[n & 0xf];
>+		n >>= 4;
>+	}
>+	ret[i--] = 'x';
>+	ret[i--] = '0';
>+	ret[sizeof(ret) - 1] = '\0';
>+	return &(ret[i + 1]);
>+}
> 
> static void
> ssh_sandbox_violation(int signum, siginfo_t *info, void *void_context)
> {
> 	char msg[256];
>+	extern struct monitor *pmonitor;
>+
>+	if (pmonitor == NULL)
>+		_exit(1);
>+
>+	strlcpy(msg, __func__, sizeof(msg));
>+	strlcat(msg, ": unexpected system call: arch:", sizeof(msg));
>+	strlcat(msg, ntoh(info->si_arch), sizeof(msg));
>+	strlcat(msg, " syscall:", sizeof(msg));
>+	strlcat(msg, ntoh(info->si_syscall), sizeof(msg));
>+	strlcat(msg, " addr:", sizeof(msg));
>+	strlcat(msg, ntoh((unsigned long)info->si_call_addr), sizeof(msg));
> 
>-	snprintf(msg, sizeof(msg),
>-	    "%s: unexpected system call (arch:0x%x,syscall:%d @ %p)",
>-	    __func__, info->si_arch, info->si_syscall, info->si_call_addr);
> 	mm_log_handler(SYSLOG_LEVEL_FATAL, 0, msg, pmonitor);
> 	_exit(1);
> }
>@@ -391,7 +423,7 @@ ssh_sandbox_child_debugging(void)
> 	sigaddset(&mask, SIGSYS);
> 
> 	act.sa_sigaction = &ssh_sandbox_violation;
>-	act.sa_flags = SA_SIGINFO;
>+	act.sa_flags = SA_SIGINFO | SA_RESETHAND;
> 	if (sigaction(SIGSYS, &act, NULL) == -1)
> 		fatal("%s: sigaction(SIGSYS): %s", __func__, strerror(errno));
> 	if (sigprocmask(SIG_UNBLOCK, &mask, NULL) == -1)

Flags:

djm: ok? (dtucker)

Actions: View | Diff

Attachments on bug 3478: 3615 | 3640 | 3641