rsync - Feb 2020 - [draft PATCH] whitelist support for refuse options

This adds support for whitelisting the acceptable options in the
"refuse options" setting in rsyncd.conf. It introduces "!"
as a
special option string that refuses most options and interprets
any following strings as patterns of options to allow.

For example, to allow only verbose and archive:

  refuse options = ! verbose archive

The "!" does't refuse no-iconv, but you can still refuse it and
use a whitelist if you want:

  refuse options = no-iconv ! verbose archive

It's not finished (needs tests and doc) I just wanted to see if
there'd be any interest in merging something of this shape
before I put more work into it.

My use case is setting up a restricted trust relationship by
allowing host A to ssh to host B with a forced command of
"rsync --server --daemon --config=/path/to/rsyncd.conf ." and
configuring the restictions in rsyncd.conf. I know what options
I want to use, it'd be nice to enforce that on the server side
without listing every other option in "refuse options".


---
 options.c | 114 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 82 insertions(+), 32 deletions(-)

diff --git a/options.c b/options.c
index e5b0cb68..02d1b174 100644
--- a/options.c
+++ b/options.c
@@ -1133,39 +1133,101 @@ static void set_refuse_options(char *bp)
 {
 	struct poptOption *op;
 	char *cp, shortname[2];
-	int is_wild, found_match;
+	int is_wild, found_match, whitelist_mode, archive_whitelisted;
 
 	shortname[1] = '\0';
+	whitelist_mode = 0;
+	archive_whitelisted = 0;
 
+	/* We flag options for refusal by abusing the "descrip" field of
+	 * struct poptOption (which we don't use) to temporarily store
+	 * a refuse flag. Refused options may be un-refused later in the
+	 * loop if whitelist mode is triggered. */
 	while (1) {
 		while (*bp == ' ') bp++;
 		if (!*bp)
 			break;
 		if ((cp = strchr(bp, ' ')) != NULL)
 			*cp= '\0';
-		is_wild = strpbrk(bp, "*?[") != NULL;
-		found_match = 0;
+		if (!strcmp(bp, "!")) {
+			whitelist_mode = 1;
+			for (op = long_options; ; op++) {
+				*shortname = op->shortName;
+				if (!op->longName && !*shortname)
+					break;
+				if (*shortname != 'e' && (!op->longName ||(
+				    strcmp("server", op->longName) &&
+				    strcmp("sender", op->longName) &&
+				    strcmp("no-iconv", op->longName))))
+					op->descrip = "refused";
+			}
+		} else {
+			is_wild = strpbrk(bp, "*?[") != NULL;
+			found_match = 0;
+			for (op = long_options; ; op++) {
+				*shortname = op->shortName;
+				if (!op->longName && !*shortname)
+					break;
+				if ((op->longName && wildmatch(bp, op->longName))
+				    || (*shortname && wildmatch(bp, shortname))) {
+					op->descrip = whitelist_mode ? 0 : "refused";
+					found_match = 1;
+					if (whitelist_mode && *shortname == 'a')
+						archive_whitelisted = 1;
+					if (!is_wild)
+						break;
+				}
+			}
+			if (!found_match) {
+				rprintf(FLOG, "No match for refuse-options string
\"%s\"\n",
+					bp);
+			}
+		}
+		if (!cp)
+			break;
+		*cp = ' ';
+		bp = cp + 1;
+	}
+
+	/* For the --archive option, the client sends the implied options
+	 * explicitly to the server, so if --archive is whitelisted then
+	 * we must individually whitelist the implied options as well. */
+	if (archive_whitelisted) {
 		for (op = long_options; ; op++) {
 			*shortname = op->shortName;
 			if (!op->longName && !*shortname)
 				break;
-			if ((op->longName && wildmatch(bp, op->longName))
-			    || (*shortname && wildmatch(bp, shortname))) {
-				if (op->argInfo == POPT_ARG_VAL)
-					op->argInfo = POPT_ARG_NONE;
-				op->val = (op - long_options) + OPT_REFUSED_BASE;
-				found_match = 1;
-				/* These flags are set to let us easily check
-				 * an implied option later in the code. */
-				switch (*shortname) {
-				case 'r': case 'd': case 'l': case 'p':
-				case 't': case 'g': case 'o': case 'D':
-					refused_archive_part = op->val;
-					break;
-				case 'z':
+			if (*shortname && strchr("rdlptgoD", *shortname))
+				op->descrip = 0;
+		}
+	}
+
+	/* The actual mechanics of marking options for refusal, now
+	 * that the set of refused options is finalized. */
+	for (op = long_options; ; op++) {
+		*shortname = op->shortName;
+		if (!op->longName && !*shortname)
+			break;
+		if (op->descrip) {
+			op->descrip = 0;
+			if (op->argInfo == POPT_ARG_VAL)
+				op->argInfo = POPT_ARG_NONE;
+			op->val = (op - long_options) + OPT_REFUSED_BASE;
+			/* These flags are set to let us easily check
+			 * an implied option later in the code. */
+			switch (*shortname) {
+			case 'r': case 'd': case 'l': case 'p':
+			case 't': case 'g': case 'o': case 'D':
+				refused_archive_part = op->val;
+				break;
+			case 'z':
+				if (!whitelist_mode)
 					refused_compress = op->val;
-					break;
-				case '\0':
+				break;
+			case '\0':
+				if (wildmatch("no-iconv", op->longName))
+					refused_no_iconv = op->val;
+				else if (!whitelist_mode) {
 					if (wildmatch("delete", op->longName))
 						refused_delete = op->val;
 					else if (wildmatch("delete-before", op->longName))
@@ -1178,22 +1240,10 @@ static void set_refuse_options(char *bp)
 						refused_progress = op->val;
 					else if (wildmatch("inplace", op->longName))
 						refused_inplace = op->val;
-					else if (wildmatch("no-iconv", op->longName))
-						refused_no_iconv = op->val;
-					break;
 				}
-				if (!is_wild)
-					break;
+				break;
 			}
 		}
-		if (!found_match) {
-			rprintf(FLOG, "No match for refuse-options string
\"%s\"\n",
-				bp);
-		}
-		if (!cp)
-			break;
-		*cp = ' ';
-		bp = cp + 1;
 	}
 }
 
-- 
2.17.1

Hi Nick,

Just in case you aren't already aware of it, there is a tool
that limits rsync to particular directories:

  rrsync - Restricts rsync to subdirectory declared in .ssh/authorized_keys
  https://www.samba.org/ftp/unpacked/rsync/support/rrsync

There is also a generic tool (I wrote this one) for limiting
incoming ssh commands to a fixed list of specific actual
commands (with semi-automatic learning of commands based on
observed behaviour during training mode):

  sshdo - controls which commands may be executed via incoming ssh
  http://raf.org/sshdo
  https://github.com/raforg/sshdo

And another (less easy to use) generic alternative:

  authprogs - SSH Command Authenticator
  https://github.com/daethnir/authprogs

I don't know if any of the above commands would give you
what you want but they might.

Suggestion: If the above won't do what you need, I'd suggest
different syntax. I don't like a directive that starts with
"refuse options" and then is mostly followed by the options
that are allowed. It's really saying refuse all options except
the following. I think it would be more intuitive if it
looked like:

  allowed options = verbose archive

and the presence of "allowed options" in rsyncd.conf causes
all other options (not present in that or any other "allowed
options" directive) to be disallowed. Just a thought.

A potential problem with your approach I imagine is that it
would apply equally to all local users but different users
might have different use cases. The tools mentioned above
would support handling different users distinctly.

cheers,
raf

Nick Cleaton via rsync wrote:
> This adds support for whitelisting the acceptable options in the
> "refuse options" setting in rsyncd.conf. It introduces
"!" as a
> special option string that refuses most options and interprets
> any following strings as patterns of options to allow.
> 
> For example, to allow only verbose and archive:
> 
>   refuse options = ! verbose archive
> 
> The "!" does't refuse no-iconv, but you can still refuse it
and
> use a whitelist if you want:
> 
>   refuse options = no-iconv ! verbose archive
> 
> It's not finished (needs tests and doc) I just wanted to see if
> there'd be any interest in merging something of this shape
> before I put more work into it.
> 
> My use case is setting up a restricted trust relationship by
> allowing host A to ssh to host B with a forced command of
> "rsync --server --daemon --config=/path/to/rsyncd.conf ." and
> configuring the restictions in rsyncd.conf. I know what options
> I want to use, it'd be nice to enforce that on the server side
> without listing every other option in "refuse options".

On Tue, 11 Feb 2020 at 01:36, raf via rsync <rsync at lists.samba.org>
wrote:
>   rrysnc
>   sshdo - controls which commands may be executed via incoming ssh
>   authprogs - SSH Command Authenticator
Those work for command line argument restrictions, but I like daemon
mode for restriction. It's a way to tell rsync directly "we do not
trust the client, don't let it out of this directory". It turns on
stricter checks on file paths that make it harder to escape the
restricted directory, and you get --munge-links automatically where
it's necessary to prevent issues such as
https://bugzilla.samba.org/show_bug.cgi?id=11879

Enforcing a restriction by filtering command line options and
arguments seems a bit fragile in comparison.
>   allowed options = verbose archive
>
> and the presence of "allowed options" in rsyncd.conf causes
> all other options (not present in that or any other "allowed
> options" directive) to be disallowed. Just a thought.
That would work, although it would mean a larger patch.

On Sun, Feb 9, 2020 at 2:06 PM Nick Cleaton via rsync <rsync at
lists.samba.org>
wrote:
> This adds support for whitelisting the acceptable options in the
"refuse
> options" setting in rsyncd.conf. It introduces "!" as a
special option
> string that refuses most options and interprets any following strings as
> patterns of options to allow.
>
> For example, to allow only verbose and archive:
>
>   refuse options = ! verbose archive
>
I had been thinking about doing something like this recently, so I
appreciate the patch.  I like your idea of (temporarily) using the descrip
value to keep track of what gets refused, which I also used in my
implementation. I went with a slightly different idiom of adding negated
match terms instead of using "!" to toggle into a different mode that
affects all the following match terms.

The current git now has support for things like the following:

refuse options = * !a !v

refuse options = delete-* !delete-during c

To make things easier, some vital options (such as "server" &
"dry-run" &
"no-iconv") are not matched by wild-card terms, so the above refusing
of
"*" leaves those options alone while still allowing someone who
really,
really wants to disable --dry-run the ability to do so by specifying
"dry-run" (a non-wild match) in their refuse list. The new rsyncd.conf
manpage lists the options that are not matched by wild-cards.

..wayne..
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.samba.org/pipermail/rsync/attachments/20200517/2f4675a2/attachment.htm>