mod_mailscript: Make EXEC action always safe to execute.

Originally, EXEC always executed programs on the host system,
which made the action, and the MailScript rule engine as a whole,
unsafe to allow users to directly control.

Now, EXEC only executes programs on the host for global
rules (which can only be set by the sysop). If found in
a mailbox rule, the program will be executed in an isolated
execution environment (same as isoexec, so in the container
and without networking). This allows users the flexibility
to use the ACTION, while limiting what they can do to things
they are already able to do anyways.

Because this now makes MailScript safe for users to edit directly,
the mailbox rules files are allowed to exist in both the maildir,
as before, as well as ~/.config/.rules (within the user's home directory).
The maildir location is retained since it is the only version for
non-user associated mailboxes (e.g. shared mailboxes). The user version
is useful since it is the only one the user can modify. Both versions
are executed if they exist.

Similarly, Sieve rules may exist either in the maildir or home
directory, but not both, unlike with MailScript. This is to avoid
complications within the ManageSieve protocol. For personal mailboxes,
Sieve scripts always exist in the home directory, which allows them to
now be edited within the user's container using a text editor.
For non-user mailboxes, the Sieve script exists in the maildir
as before, and although in theory can continue to be edited using
ManageSieve, I don't think this scenario has ever been possible,
and isn't now, since ManageSieve currently only facilitates editing
of personal Sieve rules for the authenticated user.

One caveat is that the symlink to the active Sieve script (or the
actual script, if it exists and isn't a symlink) remains in the
maildir. This is because symlinks contain the path of the target
as a string, and don't actually point to the inode of the target.
Thus, within the container, the symlink will not work if created
in the host, and vice versa. Additionally, showing a path on the
host within the container is undesirable, since this leaks information
to the user. As such, the symlink remains in the maildir. This means
users must use the ManageSieve script in order to change the active
script. On the upside, users won't be confused by a "weird symlink"
in the .configs directory, and they can still edit the scripts themselves
directly in the .configs directory.

Consequently, the MailScript changes are backwards-compatible,
but the Sieve changes are not fully backwards-compatible.

A few bugs from the previous commit (fef8df2)
have also been fixed.

There are a few edge cases currently:

* Non-user mailboxes (e.g. public/shared mailboxes) do not have
  users associated with them, and by extension, container environments.
  Thus, EXEC cannot be used with these mailboxes, except in global rules.
  This limitation could be worked around in the future.
* Environment variables like $HOME and shell shorthand like ~ are
  not evaluated prior to the program being launched, so full paths
  always need to be provided (BBS variables are okay). This isn't
  really specific to this change, but is important since it is
  intuitive to use such syntax in mailbox rules; currently, this doesn't work.

README.rst

@@ -162,7 +162,7 @@ A few especially important configuration files:
 
 * :code:`transfers.conf` - File transfer configuration
 
-Additionally, the MailScript rules engine uses a script file called :code:`.rules` in the user's root maildir (and :code:`before.rules` and :code:`after.rules` in the root maildir for global filtering) for manipulating messages.
+Additionally, the MailScript rules engine uses a script file called :code:`.rules` in the user's root maildir or user's :code:`~/.config` (and :code:`before.rules` and :code:`after.rules` in the root maildir for global filtering) for manipulating messages.
 A sample MailScript rules file is in :code:`configs/.rules` (though this is not a config file, but a sample rule script file).
 
 User Configuration
@@ -468,14 +468,51 @@ Currently, some capabilities, such as executing system commands or processing ou
 Although there are Sieve extensions to do this, the Sieve implementation in the BBS does not yet support this
 (or rather, the underlying library does not). Eventually the goal is to have full feature parity.
 
-Sieve rules can be edited by users directly using the ManageSieve protocol (net_sieve).
-In contrast, MailScript rules can only be modified by the sysop directly on the server. Additionally,
-MailScript allows for potentially dangerous operations out of the box, and should not normally be exposed to users.
-
 It is recommended that Sieve be used for filtering if possible, since this is a standardized and well supported protocol.
-MailScript is a nonstandard syntax that was invented purely for this software, so it is not portable anywhere else.
+MailScript is a nonstandard syntax that was invented purely for this software, so it is not portable to other mail servers.
 However, if the current Sieve implementation does not meet certain needs but MailScript does, feel free to use that as well.
-Both filtering engines can be used in conjunction with each other.
+Both filtering engines can be used in conjunction with each other, and they each have their advantages depending on
+the use case.
+
+Where do Sieve and MailScript filter scripts reside?
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Sieve rules reside in one of two locations. For personal mailboxes, they rise in :code:`~/config/*.sieve` and can
+also be edited by users directly using the ManageSieve protocol (net_sieve). For non-user mailboxes,
+they reside in the maildir.
+
+MailScript rules may reside in either a mailbox's maildir or in a user's :code:`~/.config/.rules` file. Originally,
+only the maildir version existed, and this version can only be edited by the sysop since users do not have access
+to their maildirs. Users can directly modify the version in their home directories, and both scripts are evaluated.
+The maildir version still exists because in non-user associated mailboxes (e.g. shared mailboxes), this is the only
+version that exists, as there is no corresponding home directory for the mailbox. If a maildir script exists,
+it is executed before the rules in the user's home directory.
+
+There are three passes of filtering performed:
+
+1. Pre-mailbox pass. Useful for setting default actions.
+2. Mailbox pass (only for messages that correspond to a mailbox, for example, messages accepted to relay to another server do not)
+3. Post-mailbox pass. Useful for enforcing required actions.
+
+The following are all the locations that can contain filter scripts:
+
+* Global rules (can only be modified by the sysop)
+
+  * :code:`$ROOT_MAILDIR/before.rules` - MailScript rules to run in pre-mailbox pass. Always executed.
+  * :code:`$ROOT_MAILDIR/after.rules` - MailScript rules to run in post-mailbox pass. Always executed.
+  * :code:`$ROOT_MAILDIR/before.sieve` - Sieve rules to run in pre-mailbox pass. Always executed.
+  * :code:`$ROOT_MAILDIR/after.sieve` - Sieve rules to run in post-mailbox pass. Always executed.
+
+* Mailbox rules, only for messages corresponding to a mailbox
+
+  * :code:`$MAILDIR/.rules` - MailScript rules to run for mailbox. Always executed. Not user-editable.
+  * :code:`~/.config/.rules` - MailScript rules to run for mailbox. Only exists for personal mailboxes. User-editable.
+  * :code:`$MAILDIR/.sieve` - Active Sieve script (or symlink) for mailbox. Not user-editable, but for personal mailboxes, can be changed using the ManageSieve protocol.
+  * :code:`~/.config/*.sieve` - All Sieve scripts for mailbox. Only exists for personal mailboxes. User-editable, including via ManageSieve protocol.
+
+Note that :code:`$ROOT_MAILDIR` is not a real variable defined by the BBS, but here refers to the root maildir, the directory that contains all the individual mailbox maildirs.
+Likewise for :code:`$MAILDIR` referring to the mailbox's maildir. :code:`~` refers to the user's home directory.
+Finally, note that "always executed" should be interpreted as "always executed if the script exists, and unless a previous global rule terminated rules processing altogether".
 
 How do I enable spam filtering?
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

bbs/system.c

@@ -731,7 +731,9 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 		snprintf(fullpath, sizeof(fullpath), "PATH=%s", parentpath);
 	}
 
-	bbs_debug(6, "%s:%d (%s) node: %p, usenode: %d, fdin: %d, fdout: %d, filename: %s, isolated: %s\n", file, lineno, func, node, usenode, fdin, fdout, filename, e->isolated ? "yes" : "no");
+	bbs_debug(6, "%s:%d (%s) node: %p, usenode: %d, fdin: %d, fdout: %d, filename: %s, env: %s, isolated: %s, user: %s\n",
+		file, lineno, func, node, usenode, fdin, fdout, filename, envp == myenvp ? "default" : "custom", e->isolated ? "yes" : "no",
+		e->user ? bbs_username(e->user) : node && node->user ? bbs_username(node->user) : "(none)");
 	if (node && usenode && (fdin != -1 || fdout != -1)) {
 		bbs_warning("fdin/fdout should not be provided if usenode == 1 (node is preferred, fdin/fdout will be ignored)\n");
 	}
@@ -883,6 +885,7 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 			char pidbuf[15];
 			char oldroot[384 + STRLEN("/.old")], newroot[384];
 			char homedir[438];
+			struct bbs_user *user = e->user ? e->user : node->user; /* If user is overriden, use the override, otherwise, use the node's user */
 
 			if (set_limits()) {
 				_exit(errno);
@@ -905,7 +908,7 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 			if (node && envp == myenvp && bbs_transfer_available()) {
 				char *tmp;
 
-				const char *username = bbs_user_is_registered(node->user) ? bbs_username(node->user) : "guest";
+				const char *username = bbs_user_is_registered(user) ? bbs_username(user) : "guest";
 				/* Used if /root/.bashrc in rootfs contains this prompt override:
 				 * PS1='${debian_chroot:+($debian_chroot)}$BBS_USER@\h:\w\$ '
 				 */
@@ -918,10 +921,10 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 					tmp++;
 				}
 
-				if (bbs_user_is_registered(node->user)) {
+				if (bbs_user_is_registered(user)) {
 					char masterhomedir[256];
 					/* Make the user's home directory accessible within the container, at /home/${BBS_USERNAME} in the container */
-					if (bbs_transfer_home_dir(node->user->id, masterhomedir, sizeof(masterhomedir))) {
+					if (bbs_transfer_home_dir(user->id, masterhomedir, sizeof(masterhomedir))) {
 						_exit(errno);
 					}
 					snprintf(homeenv + STRLEN("HOME="), sizeof(homeenv) - STRLEN("HOME="), "/home/%s", username);
@@ -952,7 +955,7 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 						SYSCALL_OR_DIE(mount, publichome, publicroot, "bind", MS_BIND | MS_REC | MS_RDONLY, NULL);
 						SYSCALL_OR_DIE(mount, publichome, publicroot, "bind", MS_REMOUNT | MS_BIND | MS_REC | MS_RDONLY, NULL);
 					}
-					if (!bbs_user_is_registered(node->user)) {
+					if (!bbs_user_is_registered(user)) {
 						/* If it's guest access, the user doesn't have a home directory,
 						 * so just make it the /home/public directory, which is better than nothing.
 						 * We make it /home/public instead of /home, so that all of the "relevant"
@@ -990,7 +993,9 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 			 * an inside the container, rm -rf .old errors with "Device or resource busy". */
 			rmdir(oldroot); /* There is an empty /.old left behind, get rid of it as it's not needed anymore */
 
-			/* Shells will automatically default to our home directory,
+			/* cd to the home directory; this way, if this is launching a shell session,
+			 * it's a better user experience. Only makes sense to do this after we've changed the root.
+			 * Shells will automatically default to our home directory,
 			 * but other programs may not. Move to that directory now, if defined. */
 			if (myenvp[3]) {
 				const char *startdir = myenvp[3] + STRLEN("HOME=");
@@ -999,9 +1004,6 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 
 			if (node && envp == myenvp && display_motd) {
 				FILE *fp;
-				/* cd to the home directory; this way, if this is launching a shell session,
-				 * it's a better user experience. Only makes sense to do this after we've changed the root. */
-				SYSCALL_OR_DIE(chdir, homeenv + STRLEN("HOME="));
 				/* We also have to handle the motd (Message of the Day).
 				 * The shell does not display the MOTD, the login program does after login before spawning the shell.
 				 * So, if there's an /etc/motd in the container, display its contents before we actually call exec.
@@ -1120,6 +1122,17 @@ int __bbs_execvpe(struct bbs_node *node, struct bbs_exec_params *e, const char *
 		bbs_node_update_winsize(node, -1, -1); /* Call with -1 as args to simply send a SIGWINCH using existing dimensions. */
 	}
 
+	if (e->priority) {
+		/* Set (reduce, typically) the priority of the child process.
+		 * This way, even if users are able to manipulate it directly,
+		 * they can't take over all system resources.
+		 *
+		 * For control, we do this in the parent rather than the child. */
+		if (setpriority(PRIO_PGRP, (id_t) pid, e->priority)) {
+			bbs_error("Failed to set priority of process group %d to %d: %s\n", pid, e->priority, strerror(errno));
+		}
+	}
+
 	bbs_debug(5, "Waiting for process %d to exit\n", pid);
 	waitpidexit(pid, filename, &res);
 	if (res == 1) {

bbs/transfer.c

@@ -26,7 +26,6 @@
 #include "include/node.h" /* for node->user */
 #include "include/user.h"
 #include "include/utils.h"
-#include "include/system.h"
 
 /*!
  * \note One thing I did not like about the original transfer implementation
@@ -129,7 +128,11 @@ int bbs_transfer_operation_allowed(struct bbs_node *node, int operation, const c
 	}
 
 	required_priv = privs[operation];
-	bbs_debug(9, "Operation %d allowed for '%s'\n", operation, diskpath);
+	if (diskpath) {
+		bbs_debug(9, "Operation %d allowed for '%s'\n", operation, diskpath);
+	} else {
+		bbs_debug(9, "Operation %d allowed\n", operation);
+	}
 	return bbs_user_priv(node->user) >= required_priv;
 }
 
@@ -197,20 +200,9 @@ int transfer_make_longname(const char *file, struct stat *st, char *buf, size_t
 	return snprintf(p, len - (size_t) (p - buf), " %s %s", modtime, file);
 }
 
-static int recursive_copy(const char *srcfiles, const char *dest)
-{
-	struct bbs_exec_params x;
-	/* It can probably do a better job than we can */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdiscarded-qualifiers"
-#pragma GCC diagnostic ignored "-Wcast-qual"
-	/* no clobber, just in case it already existed (which it shouldn't, but just supposing),
-	 * we don't want to overwrite all the user's existing files. */
-	char *const argv[] = { "cp", "-r", "-n", (char*) srcfiles, (char*) dest, NULL };
-#pragma GCC diagnostic pop
-	EXEC_PARAMS_INIT_HEADLESS(x);
-	return bbs_execvp(NULL, &x, argv[0], argv);
-}
+/* no clobber, just in case it already existed (which it shouldn't, but just supposing),
+ * we don't want to overwrite all the user's existing files. */
+#define recursive_copy(srcfiles, dest) bbs_copy_files(srcfiles, dest, COPY_RECURSIVE)
 
 int bbs_transfer_home_dir(unsigned int userid, char *buf, size_t len)
 {

bbs/utils.c

@@ -41,6 +41,7 @@
 #include "include/node.h" /* use bbs_poll_read */
 #include "include/user.h"
 #include "include/base64.h"
+#include "include/system.h"
 
 char *bbs_uuid(void)
 {
@@ -916,6 +917,31 @@ int bbs_copy_file(int srcfd, int destfd, int start, int bytes)
 	return copied;
 }
 
+int bbs_copy_files(const char *source, const char *dest, enum bbs_copy_flags flags)
+{
+	struct bbs_exec_params x;
+	char *argv[6];
+	int a = 0;
+	/* It can probably do a better job than we can */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wcast-qual"
+	/* no clobber, just in case it already existed (which it shouldn't, but just supposing),
+	 * we don't want to overwrite all the user's existing files. */
+	argv[a++] = (char*) "cp";
+	if (flags & COPY_RECURSIVE) {
+		argv[a++] = (char*) "-r";
+	}
+	if (!(flags & COPY_CLOBBER)) {
+		argv[a++] = (char*) "-n";
+	}
+	argv[a++] = (char*) source;
+	argv[a++] = (char*) dest;
+	argv[a] = NULL;
+#pragma GCC diagnostic pop
+	EXEC_PARAMS_INIT_HEADLESS(x);
+	return bbs_execvp(NULL, &x, argv[0], argv) ? -1 : 0;
+}
+
 ssize_t bbs_splice(int fd_in, int fd_out, size_t len)
 {
 	/* Use splice(2) if available, otherwise fall back to sendfile(2) */
@@ -924,13 +950,15 @@ ssize_t bbs_splice(int fd_in, int fd_out, size_t len)
 	/* off_in must be NULL if fd_in is a pipe */
 	ssize_t res, written = 0;
 	while (len > 0) {
+		/* off_out (arg 4) is NULL since fd_out is assumed to be a pipe */
 		res = splice(fd_in, &off_in, fd_out, NULL, len, 0);
 		if (res < 0) {
 			bbs_error("splice failed: %s\n", strerror(errno));
 			return -1;
 		}
 		len -= (size_t) res;
 		off_in += (off64_t) res;
+		written += res;
 	}
 	return written;
 #else

configs/.rules

@@ -10,12 +10,6 @@
 # followed by the global after rules file (e.g. /home/bbs/maildir/after.rules).
 # Rule processing occurs once the entire message has been received and before delivery is attempted.
 
-# WARNING: DO NOT ALLOW USERS to directly create or modify these rules.
-# This can be dangerous due to the ability of certain actions (e.g. EXEC) to execute any arbitrary program.
-# e.g. ACTION EXEC rm -rf /home/bbs/maildir will probably succeed (bbs user needs r/w/x permissions) and this would delete everyone's mail.
-# If you create an abstraction layer to allow users to generate MailScript rules (e.g. for forwarding, junk purposes), please be sure to vet them carefully!
-# In the meantime, this is mainly intended for power users.
-
 # The basic structure of a rule is:
 # RULE
 # [0 or more MATCH conditions]. Note that it is legitimate to have a rule with 0 MATCH statements, but this is almost certainly NOT what you want, since it would match every message.
@@ -78,7 +72,11 @@
 #                       Currently, RELAY implicitly results in a DISCARD, normal message processing will not continue afterwards and no copy of the message is retained.
 # REPLY - Reply to the sent message, to the original sender
 # MOVETO - Move the message to a specified folder in the maildir, e.g. to Junk, Trash, Folder.subfolder, etc. Can be used to implement filtering. If this action is executed multiple times, the last one wins. For outgoing messages, an IMAP URI may also be used, e.g. to APPEND the sent message to a remote IMAP mailbox.
-# EXEC - Execute a system program.
+# EXEC - Execute a system program. For global rules, the program will be executed on the host system.
+#        For mailbox rules, the program will be executed in the mailbox owner's isolated container environment (same as isoexec) and thus not have access to most of the host filesystem.
+#        Public mailboxes thus currently do not support the EXEC action, except as part of global (non-mailbox) rules.
+#        WARNING: Avoid environment variables or shell shorthand, e.g. $HOME or ~, as part of the EXEC arguments. These are not evaluated by any shell prior to the program being executed.
+#        As with a cron job, assume nothing and use full paths to be explicit. BBS varibales are evaluated as normal prior to program execution.
 # NOOP - Does nothing and always returns 0. Possibly useful for debugging rule execution with debug enabled.
 
 # Variables available for use in rules:

configs/mod_mail.conf

@@ -88,7 +88,10 @@ trashdays=7     ; Number of days messages can stay in Trash before being automat
 ;webmaster = sysop
 ;hostmaster = sysop
 ;postmaster = sysop
+;newsmaster = sysop
 ;news = sysop
 ;abuse = sysop
+;root = sysop
 
 ;[email protected] = sysop
+;*@bbs.example.net = sysop

include/system.h

@@ -25,6 +25,7 @@ struct bbs_exec_params {
 	int fdin;					/* Custom file descriptor for STDIN to created process, node->fdin or -1 otherwise */
 	int fdout;					/* Custom file descriptor for STDOUT from created process, node->fdout or -1 otherwise */
 	int priority;				/* CPU priority */
+	struct bbs_user *user;		/* Override for user. If not specified, default is the node's user. */
 	unsigned int usenode:1;		/* Whether to use the node for I/O. If FALSE, node will not be used for I/O */
 	unsigned int isolated:1;	/* Whether to create the process in an isolated container */
 	/* Container parameters */

include/transfer.h

@@ -139,7 +139,7 @@ int bbs_transfer_home_config_subdir(unsigned int userid, const char *name, char
  * \param name Name of configuration file in the configuration directory. By convention, SHOULD begin with a period (.)
  * \param[out] buf
  * \param len Size of buf
- * \retval 0 if configuration file exists, -1 on failure or if file does not exist
+ * \retval 0 if configuration file exists, -1 on failure, 1 if file does not exist
  */
 int bbs_transfer_home_config_file(unsigned int userid, const char *name, char *buf, size_t len);
 

include/utils.h

@@ -267,6 +267,20 @@ FILE *bbs_mkftemp(char *template, mode_t mode);
  */
 int bbs_copy_file(int srcfd, int destfd, int start, int bytes); /* gcc has fd_arg attributes, but not widely supported yet */
 
+enum bbs_copy_flags {
+	COPY_RECURSIVE = (1 << 0), /* Whether to copy recursively (e.g. cp -r) */
+	COPY_CLOBBER = (1 << 1), /* Whether to allow clobbering of existing files (inverse of cp -n) */
+};
+
+/*!
+ * \brief Copy file(s) to another location, by filename (equivalent of cp command)
+ * \param source Full path to source file
+ * \param dest Full path where destination file should be created
+ * \param flags Any flags for copy operation
+ * \retval 0 on success, -1 on failure
+ */
+int bbs_copy_files(const char *source, const char *dest, enum bbs_copy_flags flags);
+
 /*!
  * \brief Efficiently copy data between two file descriptors.
  * \param fd_in Input fd. Must NOT be a pipe.

include/version.h

@@ -15,4 +15,4 @@
 
 #define BBS_MAJOR_VERSION 0
 #define BBS_MINOR_VERSION 7
-#define BBS_PATCH_VERSION 0
+#define BBS_PATCH_VERSION 1