[scponly] Request review for patch to add support for bbcp to scponly
Craig Tierney
Craig.Tierney at noaa.gov
Thu Jun 4 18:00:25 EDT 2009
Kaleb Pederson wrote:
> On Thursday 04 June 2009 01:49:43 pm you wrote:
>> The -T and -S options are never passed over the wire to the remote side. The local process parses them and decides how ssh
>> should be started. If the defaults are overriden on the command line with -T and -S, those are just options to ssh.
>
> Right. The problem is that somebody may decide to invoke bbcp manually:
>
> $ ssh user at host "bbcp -S '/bin/badprogram' ..."
>
> The user could use that as a way to attempt to invoke a program they've uploaded to the chroot.
>
> In thinking about it a bit more, I don't think we need -T. '-T', when run on the scponly system, would affect the system it's
> connecting to. '-T' on the system connecting to scponly, will result in a different command attempting to be executed on the
> server, so scponly should pick it up. At least, that's what I'd venture from reading the manpage.
>
The reason there are both a -S option and -T option is because bbcp can be run from a third party where the file is remote
for both the source and destination. If used correctly, the options to those commands are for SSH, and not for BBCP. In
your example above where bbcp is used incorrectly (or maliciously) could there be a problem. We can't capture the options
before the call to bbcp, so we can't worry about the -S and -T. After the bbcp, we can do what I have below to block all
options.
For dealing with options that shouldn't be passed, what about:
diff -urN scponly-4.8/ ../scponly-4.8-bbcp/
diff -urN scponly-4.8/scponly.c ../scponly-4.8-bbcp/scponly.c
--- scponly-4.8/scponly.c 2009-05-21 19:07:02.125986000 +0000
+++ ../scponly-4.8-bbcp/scponly.c 2009-06-04 21:31:31.809460000 +0000
@@ -742,7 +742,7 @@
av[0] = substitute_known_path(av[0]);
/*
- * we only process wildcards for scp commands
+ * we only process wildcards for scp commands and bbcp
*/
#ifdef ENABLE_WILDCARDS
#ifdef ENABLE_SCP2
@@ -767,6 +767,15 @@
#endif
+ /* For BBCP, the only options that can ever be passed are SNK or SRC. So
+ check for validity of those, and exit otherwise */
+#ifdef ENABLE_BBCP
+ if (!((exact_match(av[1],"SNK") || exact_match(av[1],"SRC")) && (av[2]==NULL))) {
+ syslog(LOG_ERR,"requested command (%s) tried to use arguments other than SRC and SNK",av[0]);
+ exit(EXIT_FAILURE);
+ }
+#endif
+
flat_request = flatten_vector(av);
/*
What it does is validate that the only option passed is SNK or SRC, and no other options are passed.
I could have tried to add the options listed in BBCP into the dangerous_args list, but the problem
with that is that if BBCP ever changed, scponly wouldn't know about the options. This way, if BBCP
changes and decides to send options on the command line to the scponly server, then the bbcp call will fail.
> With respect to the note about ps, I would like to know what it uses ps for. What's the benefit / cost of having or not having
> ps present on the system.
>
The call to ps is to look up the grandparent of the active process. This may be the ssh process,
but I am not sure. I think that BBCP creates a monitor on that process so that in case it dies,
that BBCP will shut itself down. I am not positive though. I have emailed the developer of BBCP
to confirm.
Craig
> Thanks for investigating.
>
> --Kaleb
>
>
>> BBCP never tries to execute anything but:
>>
>> ssh $ARGS $HOST bbcp SRC
>>
>> or
>>
>> ssh $ARGS $HOST bbcp SNK
>>
>> If you use -T or -S, then it replaces the "ssh $ARGS" section of the command. The options to bbcp don't change.
>>
>> Should I worry that something besides SRC or SNK are passed to the other side? It doesn't fit in the getopts format, but I
>> could write something specific and wrap it in #define.
>>
>> Craig
>>
>>
>>
>>> --Kaleb
>>>
>>> On Thursday 21 May 2009 12:26:29 pm Craig Tierney wrote:
>>>> I have written a patch to scponly-4.8 so that it can support bbcp. Bbcp (http://www.slac.stanford.edu/~abh/bbcp/) is a
>>>> high performance transfer mechanism that relies on ssh for authentication and control, but creates its own channels
>>>> (multi-threaded) for bulk data transfer. Bbcp gets around the known problems with high-latency, high-bandwidth transfers
>>>> that are present in scp.
>>>>
>>>> The local bbcp calls ssh in the following manner:
>>>>
>>>> ssh $SSHOPTS $HOSTNAME bbcp (SNK|SRC)
>>>>
>>>> The SNK and SRC text defines which way the channels of the sessions should be created. As far as I can tell, all other
>>>> communication and configuration is passed through the ssh channel.
>>>>
>>>> Bbcp does call one system tool, /bin/ps. Code has been added to support this. My biggest concern with this (since I am not
>>>> security expert) is that if you want to use bbcp with a jailed-root environment, you need to mount /proc in the
>>>> jailed-root. That filesystem is mostly used for reading system data, however if root access was gained in the jailed-root,
>>>> then I could see an exploit where any entries in /proc that are writable, the use could write values that could harm or
>>>> corrupt the system.
>>>>
>>>> The patch includes changes to config.h.in and configure.in as well as changes to the code. The new feature is enabled with
>>>> --enable-bbcp-compat. I would appreciate it if someone more knowledgeable about scponly than I to review the patch below
>>>> and see if it looks correct or if I did something "horribly wrong".
>>>>
>>>> Thanks, Craig
>>>>
>>>> diff -urN scponly-4.8/config.h.in ../scponly-4.8-bbcp/config.h.in --- scponly-4.8/config.h.in 2008-01-15
>>>> 06:26:13.000000000 +0000 +++ ../scponly-4.8-bbcp/config.h.in 2009-05-21 18:43:53.990556000 +0000 @@ -14,6 +14,7 @@
>>>> #undef PASSWD_COMPAT #undef ENABLE_SCP2 #undef ENABLE_SFTP +#undef ENABLE_BBCP #undef SVNSERV_COMPAT #undef
>>>> ENABLE_WILDCARDS #undef RESTRICTIVE_FILENAMES @@ -51,6 +52,11 @@ #define PROG_CD "cd" #endif /*ENABLE_SCP2*/
>>>>
>>>> +#ifdef ENABLE_BBCP +#undef PROG_BBCP +#undef PROG_PS +#endif /*ENABLE_BBCP*/ + /* sftp logging compatibility mode */
>>>> #undef SFTP_LOGGING
>>>>
>>>> diff -urN scponly-4.8/configure.in ../scponly-4.8-bbcp/configure.in --- scponly-4.8/configure.in 2008-01-15
>>>> 06:26:13.000000000 +0000 +++ ../scponly-4.8-bbcp/configure.in 2009-05-21 18:57:03.645227000 +0000 @@ -104,6 +104,17 @@
>>>> scponly_sftp_compat=1 ])
>>>>
>>>> +AC_ARG_ENABLE([bbcp-compat], + AC_HELP_STRING([--enable-bbcp-compat], [enable bbcp compatibility]), + [ +
>>>> if test "x$enableval" != "xno"; then + bbcp_compat=1 + AC_DEFINE([ENABLE_BBCP])
>>>> + fi + ],[ + echo dnl Defaults to off, must be turned on explicitly + ]) +
>>>> AC_ARG_ENABLE([winscp-compat], AC_HELP_STRING([--enable-winscp-compat], [enable winscp (and scp) compatibility]), [ @@
>>>> -244,6 +255,13 @@ SCPONLY_PATH_PROG_DEFINE([PROG_RMDIR], [rmdir], [/bin:/usr/bin:/sbin:/usr/sbin]) fi
>>>>
>>>> +#Add options for bbcp +if test "x$enable_bbcp_compat" != "x"; then + AC_MSG_NOTICE([enabling bbcp compatability...])
>>>> + SCPONLY_PATH_PROG_DEFINE([PROG_BBCP], [bbcp], [/bin:/usr/bin]) + SCPONLY_PATH_PROG_DEFINE([PROG_PS], [ps],
>>>> [/bin:/usr/bin]) +fi + dnl Check for binaries required by the WinSCP compatibility mode dnl winscp-compat conditionals: if
>>>> test "x$enable_winscp_compat" != "xno"; then diff -urN scponly-4.8/scponly.c ../scponly-4.8-bbcp/scponly.c ---
>>>> scponly-4.8/scponly.c 2008-01-15 06:28:24.000000000 +0000 +++ ../scponly-4.8-bbcp/scponly.c 2009-05-21
>>>> 19:03:29.733811000 +0000 @@ -62,6 +62,11 @@ { PROG_RSYNC, 1 }, #endif /*ENABLE_RSYNC*/
>>>>
>>>> +#ifdef ENABLE_BBCP + { PROG_BBCP, 1 }, + { PROG_PS, 1 }, +#endif /*ENABLE_BBCP*/ + #ifdef PASSWD_COMPAT {
>>>> PROG_PASSWD, 1 }, #endif /*ENABLE_PASSWD*/ @@ -744,6 +749,10 @@ if (exact_match(av[0],PROG_SCP)) av = expand_wildcards(av);
>>>> #endif +#ifdef ENABLE_BBCP + if (exact_match(av[0],PROG_BBCP)) + av = expand_wildcards(av); +#endif
>>>> #endif
>>>>
>>>> /*
>>>>
>>>>
>>>>
>>> _______________________________________________ scponly mailing list scponly at lists.ccs.neu.edu
>>> https://lists.ccs.neu.edu/bin/listinfo/scponly
>>
>
--
Craig Tierney (craig.tierney at noaa.gov)
More information about the scponly
mailing list