From acdb6d369055d1086d88a896802c2c7f61e259f4 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Tue, 26 Aug 2014 12:07:57 -0600 Subject: [PATCH] Change behavior when plugin I/O logging function returns 0 or -1. For -1 (error) return, we now kill the command and disable the I/O logging function that returned the error. For a 0 (reject) return, we no longer display the rejected output to the user's terminal. The plugin API revision is now 1.6. --- doc/sudo_plugin.cat | 43 ++++++++++++++---- doc/sudo_plugin.man.in | 61 +++++++++++++++++++++----- doc/sudo_plugin.mdoc.in | 60 ++++++++++++++++++++----- include/sudo_plugin.h | 2 +- src/exec_pty.c | 97 ++++++++++++++++++++++++++++++++++------- 5 files changed, 214 insertions(+), 49 deletions(-) diff --git a/doc/sudo_plugin.cat b/doc/sudo_plugin.cat index b1f1371e5..36d52d0a8 100644 --- a/doc/sudo_plugin.cat +++ b/doc/sudo_plugin.cat @@ -826,6 +826,18 @@ DDEESSCCRRIIPPTTIIOONN is to be performed. If the open function returns 0, no I/O will be sent to the plugin. + If a logging function returns an error (-1), the running command will be + terminated and all of the plugin's logging functions will be disabled. + Other I/O logging plugins will still receive any remaining input or + output that has not yet been processed. + + If an input logging function rejects the data by returning 0, the command + will be terminated and the data will not be passed to the command, though + it will still be sent to any other I/O logging plugins. If an output + logging function rejects the data by returning 0, the command will be + terminated and the data will not be written to the terminal, though it + will still be sent to any other I/O logging plugins. + The io_plugin struct has the following fields: type The type field should always be set to SUDO_IO_PLUGIN. @@ -842,9 +854,10 @@ DDEESSCCRRIIPPTTIIOONN char * const user_info[], int argc, char * const argv[], char * const user_env[], char * const plugin_options[]); - The ooppeenn() function is run before the lloogg__iinnppuutt(), lloogg__oouuttppuutt() or - sshhooww__vveerrssiioonn() functions are called. It is only called if the - version is being requested or the cchheecckk__ppoolliiccyy() function has + The ooppeenn() function is run before the lloogg__ttttyyiinn(), lloogg__ttttyyoouutt(), + lloogg__ssttddiinn(), lloogg__ssttddoouutt(), lloogg__ssttddeerrrr(), or sshhooww__vveerrssiioonn() + functions are called. It is only called if the version is being + requested or if the policy plugin's cchheecckk__ppoolliiccyy() function has returned successfully. It returns 1 on success, 0 on failure, -1 if a general error occurred, or -2 if there was a usage error. In the latter case, ssuuddoo will print a usage message before it exits. @@ -965,7 +978,7 @@ DDEESSCCRRIIPPTTIIOONN allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data is rejected (which will - terminate the command) or -1 if an error occurred. + terminate the running command) or -1 if an error occurred. The function arguments are as follows: @@ -981,7 +994,7 @@ DDEESSCCRRIIPPTTIIOONN allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is rejected (which will - terminate the command) or -1 if an error occurred. + terminate the running command) or -1 if an error occurred. The function arguments are as follows: @@ -998,7 +1011,8 @@ DDEESSCCRRIIPPTTIIOONN command. This allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data is rejected - (which will terminate the command) or -1 if an error occurred. + (which will terminate the running command) or -1 if an error + occurred. The function arguments are as follows: @@ -1015,7 +1029,8 @@ DDEESSCCRRIIPPTTIIOONN output. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is rejected - (which will terminate the command) or -1 if an error occurred. + (which will terminate the running command) or -1 if an error + occurred. The function arguments are as follows: @@ -1032,7 +1047,7 @@ DDEESSCCRRIIPPTTIIOONN error. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is rejected (which - will terminate the command) or -1 if an error occurred. + will terminate the running command) or -1 if an error occurred. The function arguments are as follows: @@ -1445,6 +1460,16 @@ PPLLUUGGIINN AAPPII CCHHAANNGGEELLOOGG Version 1.5 (sudo 1.8.9) The _p_r_e_s_e_r_v_e___f_d_s entry was added to the command_info list. + Version 1.6 (sudo 1.8.11) + The behavior when an I/O logging plugin returns an error (-1) has + changed. Previously, the ssuuddoo front end took no action when the + lloogg__ttttyyiinn(), lloogg__ttttyyoouutt(), lloogg__ssttddiinn(), lloogg__ssttddoouutt(), or + lloogg__ssttddeerrrr() function returned an error. + + The behavior when an I/O logging plugin returns 0 has changed. + Previously, output from the command would be displayed to the + terminal even if an output logging function returned 0. + SSEEEE AALLSSOO sudo.conf(4), sudoers(4), sudo(1m) @@ -1464,4 +1489,4 @@ DDIISSCCLLAAIIMMEERR file distributed with ssuuddoo or http://www.sudo.ws/sudo/license.html for complete details. -Sudo 1.8.11 December 20, 2013 Sudo 1.8.11 +Sudo 1.8.11 August 25, 2014 Sudo 1.8.11 diff --git a/doc/sudo_plugin.man.in b/doc/sudo_plugin.man.in index 8db7dc151..3cdea1c58 100644 --- a/doc/sudo_plugin.man.in +++ b/doc/sudo_plugin.man.in @@ -1,7 +1,7 @@ .\" DO NOT EDIT THIS FILE, IT IS NOT THE MASTER! .\" IT IS GENERATED AUTOMATICALLY FROM sudo_plugin.mdoc.in .\" -.\" Copyright (c) 2009-2013 Todd C. Miller +.\" Copyright (c) 2009-2014 Todd C. Miller .\" .\" Permission to use, copy, modify, and distribute this software for any .\" purpose with or without fee is hereby granted, provided that the above @@ -16,7 +16,7 @@ .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.TH "SUDO_PLUGIN" "5" "December 20, 2013" "Sudo @PACKAGE_VERSION@" "OpenBSD Programmer's Manual" +.TH "SUDO_PLUGIN" "5" "August 25, 2014" "Sudo @PACKAGE_VERSION@" "OpenBSD Programmer's Manual" .nh .if n .ad l .SH "NAME" @@ -1419,6 +1419,20 @@ Any of the logging functions may be set to the pointer if no logging is to be performed. If the open function returns 0, no I/O will be sent to the plugin. .PP +If a logging function returns an error +(\-1), +the running command will be terminated and all of the plugin's logging +functions will be disabled. +Other I/O logging plugins will still receive any remaining +input or output that has not yet been processed. +.PP +If an input logging function rejects the data by returning 0, the +command will be terminated and the data will not be passed to the +command, though it will still be sent to any other I/O logging plugins. +If an output logging function rejects the data by returning 0, the +command will be terminated and the data will not be written to the +terminal, though it will still be sent to any other I/O logging plugins. +.PP The io_plugin struct has the following fields: .TP 6n type @@ -1452,15 +1466,18 @@ int (*open)(unsigned int version, sudo_conv_t conversation, The \fBopen\fR() function is run before the -\fBlog_input\fR(), -\fBlog_output\fR() +\fBlog_ttyin\fR(), +\fBlog_ttyout\fR(), +\fBlog_stdin\fR(), +\fBlog_stdout\fR(), +\fBlog_stderr\fR(), or \fBshow_version\fR() functions are called. -It is only called if the version is being requested or the +It is only called if the version is being requested or if the +policy plugin's \fBcheck_policy\fR() -function has -returned successfully. +function has returned successfully. It returns 1 on success, 0 on failure, \-1 if a general error occurred, or \-2 if there was a usage error. In the latter case, @@ -1716,7 +1733,8 @@ the user but before it is passed to the running command. This allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data -is rejected (which will terminate the command) or \-1 if an error occurred. +is rejected (which will terminate the running command) or \-1 if an +error occurred. .sp The function arguments are as follows: .TP 6n @@ -1747,7 +1765,7 @@ the command but before it is written to the user's terminal. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is rejected -(which will terminate the command) or \-1 if an error occurred. +(which will terminate the running command) or \-1 if an error occurred. .sp The function arguments are as follows: .TP 6n @@ -1780,7 +1798,7 @@ before it is passed to the running command. This allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .sp The function arguments are as follows: .TP 6n @@ -1813,7 +1831,7 @@ it is written to the standard output. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .sp The function arguments are as follows: .TP 6n @@ -1846,7 +1864,7 @@ is written to the standard error. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .sp The function arguments are as follows: .TP 6n @@ -2593,6 +2611,25 @@ The entry was added to the \fRcommand_info\fR list. +.TP 6n +Version 1.6 (sudo 1.8.11) +The behavior when an I/O logging plugin returns an error +(\-1) +has changed. +Previously, the +\fBsudo\fR +front end took no action when the +\fBlog_ttyin\fR(), +\fBlog_ttyout\fR(), +\fBlog_stdin\fR(), +\fBlog_stdout\fR(), +or +\fBlog_stderr\fR() +function returned an error. +.sp +The behavior when an I/O logging plugin returns 0 has changed. +Previously, output from the command would be displayed to the +terminal even if an output logging function returned 0. .SH "SEE ALSO" sudo.conf(@mansectform@), sudoers(@mansectform@), diff --git a/doc/sudo_plugin.mdoc.in b/doc/sudo_plugin.mdoc.in index c75119737..3452e4bbe 100644 --- a/doc/sudo_plugin.mdoc.in +++ b/doc/sudo_plugin.mdoc.in @@ -1,5 +1,5 @@ .\" -.\" Copyright (c) 2009-2013 Todd C. Miller +.\" Copyright (c) 2009-2014 Todd C. Miller .\" .\" Permission to use, copy, modify, and distribute this software for any .\" purpose with or without fee is hereby granted, provided that the above @@ -14,7 +14,7 @@ .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd December 20, 2013 +.Dd August 25, 2014 .Dt SUDO_PLUGIN @mansectform@ .Os Sudo @PACKAGE_VERSION@ .Sh NAME @@ -1255,6 +1255,20 @@ Any of the logging functions may be set to the pointer if no logging is to be performed. If the open function returns 0, no I/O will be sent to the plugin. .Pp +If a logging function returns an error +.Pq \-1 , +the running command will be terminated and all of the plugin's logging +functions will be disabled. +Other I/O logging plugins will still receive any remaining +input or output that has not yet been processed. +.Pp +If an input logging function rejects the data by returning 0, the +command will be terminated and the data will not be passed to the +command, though it will still be sent to any other I/O logging plugins. +If an output logging function rejects the data by returning 0, the +command will be terminated and the data will not be written to the +terminal, though it will still be sent to any other I/O logging plugins. +.Pp The io_plugin struct has the following fields: .Bl -tag -width 4n .It type @@ -1283,15 +1297,18 @@ int (*open)(unsigned int version, sudo_conv_t conversation, The .Fn open function is run before the -.Fn log_input , -.Fn log_output +.Fn log_ttyin , +.Fn log_ttyout , +.Fn log_stdin , +.Fn log_stdout , +.Fn log_stderr , or .Fn show_version functions are called. -It is only called if the version is being requested or the +It is only called if the version is being requested or if the +policy plugin's .Fn check_policy -function has -returned successfully. +function has returned successfully. It returns 1 on success, 0 on failure, \-1 if a general error occurred, or \-2 if there was a usage error. In the latter case, @@ -1517,7 +1534,8 @@ the user but before it is passed to the running command. This allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data -is rejected (which will terminate the command) or \-1 if an error occurred. +is rejected (which will terminate the running command) or \-1 if an +error occurred. .Pp The function arguments are as follows: .Bl -tag -width 4n @@ -1540,7 +1558,7 @@ the command but before it is written to the user's terminal. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is rejected -(which will terminate the command) or \-1 if an error occurred. +(which will terminate the running command) or \-1 if an error occurred. .Pp The function arguments are as follows: .Bl -tag -width 4n @@ -1565,7 +1583,7 @@ before it is passed to the running command. This allows the plugin to reject data if it chooses to (for instance if the input contains banned content). Returns 1 if the data should be passed to the command, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .Pp The function arguments are as follows: .Bl -tag -width 4n @@ -1590,7 +1608,7 @@ it is written to the standard output. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .Pp The function arguments are as follows: .Bl -tag -width 4n @@ -1615,7 +1633,7 @@ is written to the standard error. This allows the plugin to reject data if it chooses to (for instance if the output contains banned content). Returns 1 if the data should be passed to the user, 0 if the data is -rejected (which will terminate the command) or \-1 if an error occurred. +rejected (which will terminate the running command) or \-1 if an error occurred. .Pp The function arguments are as follows: .Bl -tag -width 4n @@ -2265,6 +2283,24 @@ The entry was added to the .Li command_info list. +.It Version 1.6 (sudo 1.8.11) +The behavior when an I/O logging plugin returns an error +.Pq \-1 +has changed. +Previously, the +.Nm sudo +front end took no action when the +.Fn log_ttyin , +.Fn log_ttyout , +.Fn log_stdin , +.Fn log_stdout , +or +.Fn log_stderr +function returned an error. +.Pp +The behavior when an I/O logging plugin returns 0 has changed. +Previously, output from the command would be displayed to the +terminal even if an output logging function returned 0. .El .Sh SEE ALSO .Xr sudo.conf @mansectform@ , diff --git a/include/sudo_plugin.h b/include/sudo_plugin.h index b8f1f7c2d..5e95df66f 100644 --- a/include/sudo_plugin.h +++ b/include/sudo_plugin.h @@ -19,7 +19,7 @@ /* API version major/minor */ #define SUDO_API_VERSION_MAJOR 1 -#define SUDO_API_VERSION_MINOR 5 +#define SUDO_API_VERSION_MINOR 6 #define SUDO_API_MKVERSION(x, y) ((x << 16) | y) #define SUDO_API_VERSION SUDO_API_MKVERSION(SUDO_API_VERSION_MAJOR, SUDO_API_VERSION_MINOR) diff --git a/src/exec_pty.c b/src/exec_pty.c index e6c97525d..21f2935ef 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -76,16 +76,22 @@ # define winsize ttysize #endif +/* + * I/O buffer with associated read/write events and a logging action. + * Used to, e.g. pass data from the pty to the user's terminal + * and any I/O logging plugins. + */ +struct io_buffer; +typedef bool (*sudo_io_action_t)(const char *, unsigned int, struct io_buffer *); struct io_buffer { SLIST_ENTRY(io_buffer) entries; struct sudo_event *revent; struct sudo_event *wevent; - bool (*action)(const char *buf, unsigned int len); + sudo_io_action_t action; int len; /* buffer length (how much produced) */ int off; /* write position (how much already consumed) */ char buf[32 * 1024]; }; - SLIST_HEAD(io_buffer_list, io_buffer); static char slavename[PATH_MAX]; @@ -202,7 +208,7 @@ pty_setup(uid_t uid, const char *tty, const char *utmp_user) /* Call I/O plugin tty input log method. */ static bool -log_ttyin(const char *buf, unsigned int n) +log_ttyin(const char *buf, unsigned int n, struct io_buffer *iob) { struct plugin_container *plugin; sigset_t omask; @@ -212,8 +218,13 @@ log_ttyin(const char *buf, unsigned int n) sigprocmask(SIG_BLOCK, &ttyblock, &omask); TAILQ_FOREACH(plugin, &io_plugins, entries) { if (plugin->u.io->log_ttyin) { - if (!plugin->u.io->log_ttyin(buf, n)) { - rval = false; + int rc = plugin->u.io->log_ttyin(buf, n); + if (rc <= 0) { + if (rc < 0) { + /* Error: disable plugin's I/O function. */ + plugin->u.io->log_ttyin = NULL; + } + rval = false; break; } } @@ -225,7 +236,7 @@ log_ttyin(const char *buf, unsigned int n) /* Call I/O plugin stdin log method. */ static bool -log_stdin(const char *buf, unsigned int n) +log_stdin(const char *buf, unsigned int n, struct io_buffer *iob) { struct plugin_container *plugin; sigset_t omask; @@ -235,7 +246,12 @@ log_stdin(const char *buf, unsigned int n) sigprocmask(SIG_BLOCK, &ttyblock, &omask); TAILQ_FOREACH(plugin, &io_plugins, entries) { if (plugin->u.io->log_stdin) { - if (!plugin->u.io->log_stdin(buf, n)) { + int rc = plugin->u.io->log_stdin(buf, n); + if (rc <= 0) { + if (rc < 0) { + /* Error: disable plugin's I/O function. */ + plugin->u.io->log_stdin = NULL; + } rval = false; break; } @@ -248,7 +264,7 @@ log_stdin(const char *buf, unsigned int n) /* Call I/O plugin tty output log method. */ static bool -log_ttyout(const char *buf, unsigned int n) +log_ttyout(const char *buf, unsigned int n, struct io_buffer *iob) { struct plugin_container *plugin; sigset_t omask; @@ -258,12 +274,29 @@ log_ttyout(const char *buf, unsigned int n) sigprocmask(SIG_BLOCK, &ttyblock, &omask); TAILQ_FOREACH(plugin, &io_plugins, entries) { if (plugin->u.io->log_ttyout) { - if (!plugin->u.io->log_ttyout(buf, n)) { + int rc = plugin->u.io->log_ttyout(buf, n); + if (rc <= 0) { + if (rc < 0) { + /* Error: disable plugin's I/O function. */ + plugin->u.io->log_ttyout = NULL; + } rval = false; break; } } } + if (!rval) { + /* + * I/O plugin rejected the output, delete the write event + * (user's tty) so we do not display the rejected output. + */ + sudo_debug_printf(SUDO_DEBUG_INFO, + "%s: deleting and freeing devtty wevent %p", __func__, iob->wevent); + sudo_ev_del(NULL, iob->wevent); + sudo_ev_free(iob->wevent); + iob->wevent = NULL; + iob->off = iob->len = 0; + } sigprocmask(SIG_SETMASK, &omask, NULL); debug_return_bool(rval); @@ -271,7 +304,7 @@ log_ttyout(const char *buf, unsigned int n) /* Call I/O plugin stdout log method. */ static bool -log_stdout(const char *buf, unsigned int n) +log_stdout(const char *buf, unsigned int n, struct io_buffer *iob) { struct plugin_container *plugin; sigset_t omask; @@ -281,12 +314,29 @@ log_stdout(const char *buf, unsigned int n) sigprocmask(SIG_BLOCK, &ttyblock, &omask); TAILQ_FOREACH(plugin, &io_plugins, entries) { if (plugin->u.io->log_stdout) { - if (!plugin->u.io->log_stdout(buf, n)) { + int rc = plugin->u.io->log_stdout(buf, n); + if (rc <= 0) { + if (rc < 0) { + /* Error: disable plugin's I/O function. */ + plugin->u.io->log_stdout = NULL; + } rval = false; break; } } } + if (!rval) { + /* + * I/O plugin rejected the output, delete the write event + * (user's stdout) so we do not display the rejected output. + */ + sudo_debug_printf(SUDO_DEBUG_INFO, + "%s: deleting and freeing stdout wevent %p", __func__, iob->wevent); + sudo_ev_del(NULL, iob->wevent); + sudo_ev_free(iob->wevent); + iob->wevent = NULL; + iob->off = iob->len = 0; + } sigprocmask(SIG_SETMASK, &omask, NULL); debug_return_bool(rval); @@ -294,7 +344,7 @@ log_stdout(const char *buf, unsigned int n) /* Call I/O plugin stderr log method. */ static bool -log_stderr(const char *buf, unsigned int n) +log_stderr(const char *buf, unsigned int n, struct io_buffer *iob) { struct plugin_container *plugin; sigset_t omask; @@ -304,12 +354,29 @@ log_stderr(const char *buf, unsigned int n) sigprocmask(SIG_BLOCK, &ttyblock, &omask); TAILQ_FOREACH(plugin, &io_plugins, entries) { if (plugin->u.io->log_stderr) { - if (!plugin->u.io->log_stderr(buf, n)) { + int rc = plugin->u.io->log_stderr(buf, n); + if (rc <= 0) { + if (rc < 0) { + /* Error: disable plugin's I/O function. */ + plugin->u.io->log_stderr = NULL; + } rval = false; break; } } } + if (!rval) { + /* + * I/O plugin rejected the output, delete the write event + * (user's stderr) so we do not display the rejected output. + */ + sudo_debug_printf(SUDO_DEBUG_INFO, + "%s: deleting and freeing stderr wevent %p", __func__, iob->wevent); + sudo_ev_del(NULL, iob->wevent); + sudo_ev_free(iob->wevent); + iob->wevent = NULL; + iob->off = iob->len = 0; + } sigprocmask(SIG_SETMASK, &omask, NULL); debug_return_bool(rval); @@ -504,7 +571,7 @@ io_callback(int fd, int what, void *v) default: sudo_debug_printf(SUDO_DEBUG_INFO, "read %d bytes from fd %d", n, fd); - if (!iob->action(iob->buf + iob->len, n)) + if (!iob->action(iob->buf + iob->len, n, iob)) terminate_command(cmnd_pid, true); iob->len += n; /* Enable writer if not /dev/tty or we are foreground pgrp. */ @@ -589,7 +656,7 @@ io_callback(int fd, int what, void *v) } static void -io_buf_new(int rfd, int wfd, bool (*action)(const char *, unsigned int), +io_buf_new(int rfd, int wfd, bool (*action)(const char *, unsigned int, struct io_buffer *), struct io_buffer_list *head) { int n;