Russ Allbery <ea...@eyrie.org> writes:
> I suspect that the rsync protocol is also vulnerable to a version of
> this same bug if .ssh/config is writable and is used as the ssh client
> path and the ssh binary is available on the server side, by sending an
> rsync command that tries to copy a file to localhost: similar to your
> second attack example. I believe tightening rssh's check that the rsync
> command line starts with --server would address that, since I think (but
> haven't confirmed) that will disable remote copies.
> It seems likely that there's some way of abusing cvs as well, given its
> huge command surface.
Hi all,
I finally had a chance to dig into this a little more, since I owe rssh
users in Debian stable some security support.
The short version is that I think I've patched up the obvious ways to
exploit this cluster of vulnerabilities via rsync or scp, but I'm not at
all convinced that I've found all the other routes in (and I think it's
quite likely there's some lingering way in via the cvs support, since the
attack surface for cvs is just so large).
Attached are three patches: the first adds argument checking for scp, the
second is an update of the earlier rsync patch that tightens its argument
checking and requires rsync be put into server mode (which looks like it
will prevent initiation of outbound ssh connections, which would be
another route to this same exploit), and the third applies argument
checking to commands run inside a chroot. It turns out that rssh was
never checking arguments if run via a chroot, and I didn't notice because
I personally use it as a mild defense in depth where a chroot didn't feel
important.
(This last was pointed out to me by another list member in private email.
I'm not naming you explicitly since I wasn't sure if you wanted to be
mentioned on-list, but feel free to claim credit -- that was a great
catch!)
I've uploaded a new version of the Debian package to unstable with these
fixes and will work with the Debian security team on a stable fix.
However, once the new Debian package propagates to testing, I expect to
request that it be removed (in advance of our upcoming new Debian stable
release), so it will not be in the next release of Debian. I think it's
time; we know the security model isn't the greatest, and I suspect we'll
uncover more problems like this and I don't want to provide security
support through another release cycle.
--
Russ Allbery (ea...@eyrie.org) <http://www.eyrie.org/~eagle/>
From: Russ Allbery <r...@debian.org>
Date: Sat, 7 Dec 2013 18:32:56 -0800
Subject: Handle rsync v3 -e protocol option
As of rsync 3, rsync reused the -e option to pass protocol information
from the client to the server. We therefore cannot reject all -e
options to rsync, only ones not sent with --server or containing
something other than protocol information as an argument.
Be stricter about the rsync command line and require --server as the
first argument, which disables attempts to initiate rsync outbound from
the server and in turn could trigger running code specified in ssh
client configuration options.
Also scan the rsync command line for any --rsh option and reject it as
well. This replaces and improves the upstream strategy for rejecting
that command-line option, taking advantage of the parsing added to
check the -e option.
Based on work by Robert Hardy.
Debian Bug#471803
---
util.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 68 insertions(+), 8 deletions(-)
diff --git a/util.c b/util.c
index ef1a5d8..6ee0799 100644
--- a/util.c
+++ b/util.c
@@ -56,6 +56,7 @@
#ifdef HAVE_LIBGEN_H
#include <libgen.h>
#endif /* HAVE_LIBGEN_H */
+#include <regex.h>
/* LOCAL INCLUDES */
#include "pathnames.h"
@@ -197,6 +198,69 @@ bool check_command( char *cl, ShellOptions_t *opts, char *cmd, int cmdflag )
}
+/*
+ * rsync_okay() - require --server on all rsh command lines, check that -e
+ * contains only protocol information, and reject any --rsh
+ * option. Returns FALSE if the command line should not be
+ * allowed, TRUE if it is okay.
+ */
+static int rsync_okay( char **vec )
+{
+ regex_t re;
+ int server = FALSE;
+ int e_found = FALSE;
+
+ /*
+ * rsync will send -e, followed by either just "." (meaning no special
+ * protocol) or "N.N" (meaning a pre-release protocol version),
+ * followed by some number of alphabetic flags indicating various
+ * supported options. There may be other options between - and the e,
+ * but -e will always be the last option in the string. A typical
+ * option passed by the client is "-ltpre.iL".
+ *
+ * Note that if --server is given, this should never be parsed as a
+ * shell, but we'll tightly verify it anyway, just in case.
+ *
+ * This regex matches the acceptable flags containing -e, so if it
+ * does not match, the command line should be rejected.
+ */
+ static const char pattern[]
+ = "^-[a-df-zA-Z]*e[0-9]*\\.[0-9]*[a-zA-Z]*$";
+
+ /*
+ * Only recognize --server if it's the first option. rsync itself
+ * always passes it that way, and if it's not the first argument, it
+ * could be hidden from the server as an argument to some other
+ * option.
+ */
+ if ( !(vec && vec[0] && vec[1] && strcmp(vec[1], "--server") == 0) )
+ return FALSE;
+
+ /* Check the remaining options for -e or --rsh. */
+ if ( regcomp(&re, pattern, REG_EXTENDED | REG_NOSUB) != 0 ){
+ return FALSE;
+ }
+ while (vec && *vec){
+ if ( strcmp(*vec, "--") == 0 ) break;
+ if ( strcmp(*vec, "--rsh") == 0
+ || strncmp(*vec, "--rsh=", strlen("--rsh=")) == 0 ){
+ regfree(&re);
+ return FALSE;
+ }
+ if ( strncmp(*vec, "--", 2) != 0 && opt_exist(*vec, 'e') ){
+ e_found = TRUE;
+ if ( regexec(&re, *vec, 0, NULL, 0) != 0 ){
+ regfree(&re);
+ return FALSE;
+ }
+ }
+ vec++;
+ }
+ regfree(&re);
+ return TRUE;
+}
+
+
/*
* check_command_line() - take the command line passed to rssh, and verify
* that the specified command is one the user is
@@ -230,14 +294,10 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
if ( check_command(*cl, opts, PATH_RSYNC, RSSH_ALLOW_RSYNC) ){
/* filter -e option */
- if ( opt_filter(cl, 'e') ) return NULL;
- while (cl && *cl){
- if ( strstr(*cl, "--rsh" ) ){
- fprintf(stderr, "\ninsecure --rsh= not allowed.");
- log_msg("insecure --rsh option in rsync command line!");
- return NULL;
- }
- cl++;
+ if ( !rsync_okay(cl) ){
+ fprintf(stderr, "\ninsecure rsync options not allowed.");
+ log_msg("insecure rsync options in rsync command line!");
+ return NULL;
}
return PATH_RSYNC;
}
From: Russ Allbery <r...@debian.org>
Date: Thu, 17 Jan 2019 19:21:40 -0800
Subject: Verify scp command options
ESnet discovered a security vulnerability in the scp backend for
rssh. Since the arguments to scp on the server side were not
checked, the client could pass in arbitrary scp command-line flags,
including setting arbitrary scp options. This allows setting the
option PKCS11Provider, which loads and executes code from a shared
module.
Even if the -o flag is blocked, this is still possible via -F to
load an already-uploaded ssh configuration file, or, if .ssh/config
is writable, by just uploading that configuration file directly
first.
Attempt to protect against this attack by checking the command line
of scp and only allowing the options that are passed to the server
end of the connection. Specifically, do not allow multiple
non-option arguments, which attempts to prevent causing the server
to initiate an scp command. (This will break scp -3 through rssh,
which seems like an acceptable tradeoff.)
Debian Bug#919623
---
util.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/util.c b/util.c
index 3ec01e1..6e668c9 100644
--- a/util.c
+++ b/util.c
@@ -264,6 +264,45 @@ static int rsync_okay( char **vec )
}
+/*
+ * scp_okay() - take the command line and check that it is a hopefully-safe scp
+ * server command line, accepting only very specific options.
+ * Returns FALSE if the command line should not be allowed, TRUE
+ * if it is okay.
+ */
+static int scp_okay( char **vec )
+{
+ int saw_file = FALSE;
+ int saw_end = FALSE;
+
+ for ( vec++; vec && *vec; vec++ ){
+ /* Allowed options. */
+ if ( !saw_end ) {
+ if ( strcmp(*vec, "-v") == 0 ) continue;
+ if ( strcmp(*vec, "-r") == 0 ) continue;
+ if ( strcmp(*vec, "-p") == 0 ) continue;
+ if ( strcmp(*vec, "-d") == 0 ) continue;
+ if ( strcmp(*vec, "-f") == 0 ) continue;
+ if ( strcmp(*vec, "-t") == 0 ) continue;
+ }
+
+ /* End of arguments. One more argument allowed after this. */
+ if ( !saw_end && strcmp(*vec, "--") == 0 ){
+ saw_end = TRUE;
+ continue;
+ }
+
+ /* No other options allowed, but allow file starting with -. */
+ if ( *vec[0] == '-' && !saw_end ) return FALSE;
+ if ( saw_file ) return FALSE;
+ saw_file = TRUE;
+ }
+
+ /* We must have seen a single file. */
+ return saw_file;
+}
+
+
/*
* check_command_line() - take the command line passed to rssh, and verify
* that the specified command is one the user is
@@ -279,8 +318,11 @@ char *check_command_line( char **cl, ShellOptions_t *opts )
return PATH_SFTP_SERVER;
if ( check_command(*cl, opts, PATH_SCP, RSSH_ALLOW_SCP) ){
- /* filter -S option */
- if ( opt_filter(cl, 'S') ) return NULL;
+ if ( !scp_okay(cl) ){
+ fprintf(stderr, "\ninsecure scp option not allowed.");
+ log_msg("insecure scp option in scp command line");
+ return NULL;
+ }
return PATH_SCP;
}
From: Russ Allbery <r...@debian.org>
Date: Mon, 28 Jan 2019 20:15:30 -0800
Subject: Check command line after chroot
When a command was configured with a chroot, rssh did not check
the safety of the command line after chroot, allowing various
vectors of remote code execution inside the chroot environment.
Perform the same check after chroot as is performed before running
the command when a chroot is not configured.
---
rssh_chroot_helper.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/rssh_chroot_helper.c b/rssh_chroot_helper.c
index 8a35cdc..73d8c7b 100644
--- a/rssh_chroot_helper.c
+++ b/rssh_chroot_helper.c
@@ -218,6 +218,12 @@ int main( int argc, char **argv )
ch_fatal_error("build_arg_vector()", argv[2],
"bad expansion");
+ /* check the command for safety */
+ if ( !check_command_line(argvec, &opts) ){
+ fprintf(stderr, "\n");
+ exit(1);
+ }
+
/*
* This is the old way to figure out what program to run. Since we're
* re-parsing the config file in rssh_chroot helper, we could get rid
_______________________________________________
rssh-discuss mailing list
rssh-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rssh-discuss
