Re: rssh security update breaks rsync via Synology's "hyper backup"

[Holger Levsen]

Hi Roman,

the security team is not responsible for Debian LTS, I've thus added 
debian-lts@lists.d.o to the mail recipients, so that they become aware
of your issue.

On Thu, Feb 14, 2019 at 06:06:34PM +0100, Roman Medina-Heigl Hernandez wrote:
> Hi security-fellows,
> 
> I applied recent rssh security updates to Debian 8 (jessie) and I
> noticed that it breaks Synology's "Hyper backup" tool (with rsync method).
> 
> The relevant log lines at my Debian server:
> 
> Feb 10 03:28:21 roman rssh[19985]: cmd 'rsync' approved
> Feb 10 03:28:21 roman rssh[19985]: insecure rsync options in rsync
> command line!
> Feb 10 03:28:21 roman rssh[19985]: user synology attempted to execute
> forbidden commands
> Feb 10 03:28:21 roman rssh[19985]: command: rsync --server --daemon .
> 
> Is it really unsafe to issue a "rsync --server --daemon ." command so it
> deserves to be blocked?`
> 
> 
> PS: OS info:
> 
> root@roman:~# cat /etc/debian_version
> 8.11
> root@roman:~# dpkg -l rssh   
> Deseado=desconocido(U)/Instalar/eliminaR/Purgar/retener(H)
> |
> Estado=No/Inst/ficheros-Conf/desempaqUetado/medio-conF/medio-inst(H)/espera-disparo(W)/pendienTe-disparo
> |/ Err?=(ninguno)/requiere-Reinst (Estado,Err: mayúsc.=malo)
> ||/ Nombre    Versión
> Arquitectura    Descripción
> +++-=-===-===-
> ii  rssh  2.3.4-4+deb8u2 
> amd64   Restricted shell allowing scp, sftp, cvs, svn,
> rsync or rdist
> 
> PS2: I'm not suscribed to LTS-list, but I guess the problem may be both
> in stable and oldstable versions.
> 
> Cheers,
> 
> -Román
> 

-- 
tschau,
Holger

---
   holger@(debian|reproducible-builds|layer-acht).org
   PGP fingerprint: B8BF 5413 7B09 D35C F026 FE9D 091A B856 069A AA1C

In Europe there are people prosecuted by courts because they saved other people
from drowning in the  Mediterranean Sea.  That is almost as absurd  as if there
were people being prosecuted because they save humans from drowning in the sea.


signature.asc
Description: PGP signature

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Chris Lamb]

[debian-secur...@lists.debian.org → Bcc]

Holger Levsen wrote:

> > I applied recent rssh security updates to Debian 8 (jessie) and I
> > noticed that it breaks Synology's "Hyper backup" tool (with rsync method).
> > 
> > Feb 10 03:28:21 roman rssh[19985]: cmd 'rsync' approved
> > Feb 10 03:28:21 roman rssh[19985]: insecure rsync options in rsync
> > command line!
> > Feb 10 03:28:21 roman rssh[19985]: user synology attempted to execute
> > forbidden commands
> > Feb 10 03:28:21 roman rssh[19985]: command: rsync --server --daemon .
> > 
> > Is it really unsafe to issue a "rsync --server --daemon ." command so it
> > deserves to be blocked?`

FYI this is the patch in question:

https://sources.debian.org/src/rssh/2.3.4-11/debian/patches/0007-Verify-rsync-command-options.patch/#L15-L20


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org 🍥 chris-lamb.co.uk
   `-

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Roman Medina-Heigl Hernandez]

Added Russ (rssh maintainer).

I cannot probe it but I guess chances are high that the issue is present
both in stable and oldstable (I cannot find a good reason to filter
different commands: solution should be the same or very similar) so I'm
still keeping debian-security in the loop.


PS: Thx Holger & Chris.

Cheers,

-Román


El 14/02/2019 a las 18:47, Chris Lamb escribió:
> [debian-secur...@lists.debian.org → Bcc]
>
> Holger Levsen wrote:
>
>>> I applied recent rssh security updates to Debian 8 (jessie) and I
>>> noticed that it breaks Synology's "Hyper backup" tool (with rsync method).
>>>
>>> Feb 10 03:28:21 roman rssh[19985]: cmd 'rsync' approved
>>> Feb 10 03:28:21 roman rssh[19985]: insecure rsync options in rsync
>>> command line!
>>> Feb 10 03:28:21 roman rssh[19985]: user synology attempted to execute
>>> forbidden commands
>>> Feb 10 03:28:21 roman rssh[19985]: command: rsync --server --daemon .
>>>
>>> Is it really unsafe to issue a "rsync --server --daemon ." command so it
>>> deserves to be blocked?`
> FYI this is the patch in question:
>
> https://sources.debian.org/src/rssh/2.3.4-11/debian/patches/0007-Verify-rsync-command-options.patch/#L15-L20
>
>
> Regards,
>

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Russ Allbery]

Roman Medina-Heigl Hernandez  writes:

> Added Russ (rssh maintainer).

> I cannot probe it but I guess chances are high that the issue is present
> both in stable and oldstable (I cannot find a good reason to filter
> different commands: solution should be the same or very similar) so I'm
> still keeping debian-security in the loop.

That command line exactly is probably safe, but --daemon is definitely not
safe in combination with --config, -M, or --dparam.  I'm not sure what
happens if you pass combinations like --server --daemon --address or
--port.  Unfortunately, so far as I can tell, --server --daemon is not
even documented in the rsync man page as something you can do (I certainly
didn't know about its existence before this string of CVEs), so it's
pretty hard to figure out what its intended behavior is without doing a
deep dive into source code.

We can try to make the command-line verification even more complex to try
to allow for more edge cases (another one just came up in an Ubuntu bug,
where libssh apparently sends arguments differently than scp itself does).
I'm not super-thrilled, but I guess I created this problem and signed up
for it, so I can try to keep playing whack-a-mole with new command-line
variations.

Note that I'm definitely removing rssh from unstable and testing before
the next release as unsupportable.  Upstream has orphaned it, and I think
the primitive that it's attempting to implement is fundamentally
impossible to maintain securely.  So the long-term solution is for
everyone to migrate away from it, I'm afraid; the question is what to do
about stable (and oldstable).

In this particular case, a simple command="rsync --server --daemon ." in
your ssh authorized_keys file might be a better solution than rssh.  (But
be aware of CVE-2019-3463.)

-- 
Russ Allbery (r...@debian.org)   

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Antoine Beaupré]

On 2019-02-14 10:08:40, Russ Allbery wrote:
> Roman Medina-Heigl Hernandez  writes:
>
>> Added Russ (rssh maintainer).
>
>> I cannot probe it but I guess chances are high that the issue is present
>> both in stable and oldstable (I cannot find a good reason to filter
>> different commands: solution should be the same or very similar) so I'm
>> still keeping debian-security in the loop.
>
> That command line exactly is probably safe, but --daemon is definitely not
> safe in combination with --config, -M, or --dparam.  I'm not sure what
> happens if you pass combinations like --server --daemon --address or
> --port.  Unfortunately, so far as I can tell, --server --daemon is not
> even documented in the rsync man page as something you can do (I certainly
> didn't know about its existence before this string of CVEs), so it's
> pretty hard to figure out what its intended behavior is without doing a
> deep dive into source code.
>
> We can try to make the command-line verification even more complex to try
> to allow for more edge cases (another one just came up in an Ubuntu bug,
> where libssh apparently sends arguments differently than scp itself does).
> I'm not super-thrilled, but I guess I created this problem and signed up
> for it, so I can try to keep playing whack-a-mole with new command-line
> variations.
>
> Note that I'm definitely removing rssh from unstable and testing before
> the next release as unsupportable.  Upstream has orphaned it, and I think
> the primitive that it's attempting to implement is fundamentally
> impossible to maintain securely.  So the long-term solution is for
> everyone to migrate away from it, I'm afraid; the question is what to do
> about stable (and oldstable).
>
> In this particular case, a simple command="rsync --server --daemon ." in
> your ssh authorized_keys file might be a better solution than rssh.  (But
> be aware of CVE-2019-3463.)

Thanks for the detailed analysis Russ! It does seem to be a bit of a
whack-a-mole game that would be better solved by proper use of
`command`...

That said, if we do fix this in jessie, we should do it at the same time
as the regression identified in stretch (DSA-4377-2).

Russ, do you want to handle the Jessie update or should the LTS team do
it?

Should we wait for resolution on this issue before shipping the errata?

Thanks!

-- 
In serious work commanding and discipline are of little avail.
 - Peter Kropotkin

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Moritz Muehlenhoff]

On Thu, Feb 14, 2019 at 10:08:40AM -0800, Russ Allbery wrote:
> Unfortunately, so far as I can tell, --server --daemon is not
> even documented in the rsync man page as something you can do (I certainly
> didn't know about its existence before this string of CVEs), so it's
> pretty hard to figure out what its intended behavior is without doing a
> deep dive into source code.

The rsync manpage states "The options --server and --sender are used internally
 by rsync, and should never be typed by a user under normal circumstances.", we
should not add additional complexity on top of the existing patches for the
use case at hand.

Cheers,
Moritz

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Russ Allbery]

Antoine Beaupré  writes:

> That said, if we do fix this in jessie, we should do it at the same time
> as the regression identified in stretch (DSA-4377-2).

> Russ, do you want to handle the Jessie update or should the LTS team do
> it?

> Should we wait for resolution on this issue before shipping the errata?

Apologies for the delayed reply -- I had company for the holiday weekend
in the US.

I think the regression identified at:

https://bugs.launchpad.net/ubuntu/+source/rssh/+bug/1815935

is sufficiently serious to warrant another regression fix in stable, if
the security team agrees.  I'm going to prepare a new package for
unstable and stable with a fix for that regression, and can do oldstable
at the same time and roll in the DSA-4377-2 regression fix.

While I agree that using undocumented features of rsync is a little
dubious, I'm also willing to include a fix to allow the specific command
line "rsync --server --daemon " since (a) it seems to be safe, (b)
looks easy enough to do, and (c) my only goal with rssh at this point is
to keep it working through the stable support period, so I'm not too
worried about the long-term maintenance burden of one-off hacks like that.

I should be able to do this later today.

Does this plan sound good to everyone?  I'll follow up with the proposed
diffs for stable and oldstable.

-- 
Russ Allbery (r...@debian.org)   

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Roman Medina-Heigl Hernandez]

El 18/02/2019 a las 18:27, Russ Allbery escribió:
> While I agree that using undocumented features of rsync is a little
> dubious, I'm also willing to include a fix to allow the specific command
> line "rsync --server --daemon " since (a) it seems to be safe, (b)
> looks easy enough to do, and (c) my only goal with rssh at this point is
> to keep it working through the stable support period, so I'm not too
> worried about the long-term maintenance burden of one-off hacks like that.
>
> I should be able to do this later today.
>
> Does this plan sound good to everyone?  I'll follow up with the proposed
> diffs for stable and oldstable.

If you want, shoot me with a .deb I could install in oldstable in order
to test the "rsync --server --daemon" fix.

Cheers,

-Román

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Antoine Beaupré]

On 2019-02-18 09:27:37, Russ Allbery wrote:
> Does this plan sound good to everyone?  I'll follow up with the proposed
> diffs for stable and oldstable.

Works for me (LTS), although I won't be the one performing the upgrade
(I've unclaimed the package for other reasons).

Thanks for your work!

A.

-- 
Gods don't like people not doing much work. People who aren't busy all
the time might start to think.
- Terry Pratchett, Small Gods

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Chris Lamb]

Antoine Beaupré wrote:

> > Does this plan sound good to everyone?  I'll follow up with the proposed
> > diffs for stable and oldstable.
> 
> Works for me (LTS), although I won't be the one performing the upgrade
> (I've unclaimed the package for other reasons).

Works for me too and happy to take this. Claimed package in:

  
https://salsa.debian.org/security-tracker-team/security-tracker/commit/dd5c546e66da71f4029f09337a84aadaa527dcce

Looking forward to receiving your debdiffs. :)


Regards,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org 🍥 chris-lamb.co.uk
   `-

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Russ Allbery]

Roman Medina-Heigl Hernandez  writes:
> El 18/02/2019 a las 18:27, Russ Allbery escribió:

>> While I agree that using undocumented features of rsync is a little
>> dubious, I'm also willing to include a fix to allow the specific
>> command line "rsync --server --daemon " since (a) it seems to be
>> safe, (b) looks easy enough to do, and (c) my only goal with rssh at
>> this point is to keep it working through the stable support period, so
>> I'm not too worried about the long-term maintenance burden of one-off
>> hacks like that.
>>
>> I should be able to do this later today.
>>
>> Does this plan sound good to everyone?  I'll follow up with the
>> proposed diffs for stable and oldstable.

> If you want, shoot me with a .deb I could install in oldstable in order
> to test the "rsync --server --daemon" fix.

Unfortunately, I took a closer look, and it turns out that this command
was never safe.  It also allows arbitrary code excution on the server
side if the client can write to $HOME.  This is because:

   --config=FILE
  This specifies an alternate config file than the default.  This
  is only relevant when --daemon is specified.  The default is
  /etc/rsyncd.conf unless the daemon is running over a remote
  shell program and the remote user is not the super-user; in that
  case the default is rsyncd.conf in the current directory
  (typically $HOME).

That behavior of loading rsyncd.conf from the current directory was the
piece I had missed before.

Presumably, this is exactly the behavior that Synology relies on, but this
means that if the client can write to this configuration file, it can just
include a pre-xfer exec setting in that rsyncd.conf file and run commands
on the server side.

So, unfortunately we won't be able to fix Synology in a stable update,
since it was relying on insecure behavior.  I'll continue with an update
to fix the libssh2 regression.

-- 
Russ Allbery (r...@debian.org)   

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Russ Allbery]

Russ Allbery  writes:

> I'll follow up with the proposed diffs for stable and oldstable.

Here are the proposed diffs for stable and oldstable.  The stable diff
just fixes the libssh2 interoperability regression.  The oldstable diff
fixes both that and the regression with downloading multiple files via a
wildcard.  Let me know if these are okay for upload.

I've not done an LTS security upload before, but it looks from the wiki
that it uses the same security-master process as stable security updates.
Please let me know if that's wrong.

-- 
Russ Allbery (ea...@eyrie.org)  

diff -Nru rssh-2.3.4/debian/changelog rssh-2.3.4/debian/changelog
--- rssh-2.3.4/debian/changelog	2019-02-10 11:26:30.0 -0800
+++ rssh-2.3.4/debian/changelog	2019-02-18 19:40:06.0 -0800
@@ -1,3 +1,12 @@
+rssh (2.3.4-5+deb9u4) stretch-security; urgency=high
+
+  * The fix for the scp security vulnerability in 2.3.4-9 combined with
+the regression fix in 2.3.4-10 rejected the -pf and -pt options, which
+are sent by libssh2's scp support.  Add support for those variants.
+(LP #1815935)
+
+ -- Russ Allbery   Mon, 18 Feb 2019 19:40:06 -0800
+
 rssh (2.3.4-5+deb9u3) stretch-security; urgency=high
 
   * The fix for the scp security vulneraability in 2.3.4-5+deb9u1
diff -Nru rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch
--- rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch	2019-02-10 11:26:30.0 -0800
+++ rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch	2019-02-18 19:40:06.0 -0800
@@ -17,7 +17,8 @@
 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.  Require either -f or -t be given, which
-disables scp's attempts to connect to a remote host.
+disables scp's attempts to connect to a remote host.  Allow these as
+-pf and -pt, which are sent by libssh2.
 
 Debian Bug#919623
 ---
@@ -25,7 +26,7 @@
  1 file changed, 42 insertions(+), 2 deletions(-)
 
 diff --git a/util.c b/util.c
-index dc8c8fb..71e54a5 100644
+index dc8c8fb..4203eac 100644
 --- a/util.c
 +++ b/util.c
 @@ -266,6 +266,43 @@ static int rsync_okay( char **vec )
@@ -48,11 +49,11 @@
 +		if ( strcmp(*vec, "-r") == 0 ) continue;
 +		if ( strcmp(*vec, "-p") == 0 ) continue;
 +		if ( strcmp(*vec, "-d") == 0 ) continue;
-+		if ( strcmp(*vec, "-f") == 0 ){
++		if ( strcmp(*vec, "-f") == 0 || strcmp(*vec, "-pf") == 0 ){
 +			saw_f_or_t = TRUE;
 +			continue;
 +		}
-+		if ( strcmp(*vec, "-t") == 0 ){
++		if ( strcmp(*vec, "-t") == 0 || strcmp(*vec, "-pt") == 0 ){
 +			saw_f_or_t = TRUE;
 +			continue;
 +		}
diff -Nru rssh-2.3.4/debian/changelog rssh-2.3.4/debian/changelog
--- rssh-2.3.4/debian/changelog	2019-02-01 20:28:01.0 -0800
+++ rssh-2.3.4/debian/changelog	2019-02-18 19:50:49.0 -0800
@@ -1,3 +1,15 @@
+rssh (2.3.4-4+deb8u3) jessie-security; urgency=high
+
+  * The fix for the scp security vulneraability in 2.3.4-5+deb9u1
+introduced a regression that blocked scp of multiple files from a
+server using rssh.  Based on further analysis of scp's command-line
+parsing, relax the check to require the server command contain -f or
+-t, plus the -pf and -pt variants sent by libssh2, which should
+deactivate scp's support for remote files.  (Closes: #921655)
+(LP: #1815935)
+
+ -- Russ Allbery   Mon, 18 Feb 2019 19:50:49 -0800
+
 rssh (2.3.4-4+deb8u2) jessie-security; urgency=high
 
   * Non-maintainer upload by the LTS team.
diff -Nru rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch
--- rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch	2019-02-01 20:28:01.0 -0800
+++ rssh-2.3.4/debian/patches/0009-Verify-scp-command-options.patch	2019-02-18 19:50:49.0 -0800
@@ -16,21 +16,20 @@
 
 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.)
+end of the connection.  Require either -f or -t be given, which
+disables scp's attempts to connect to a remote host.  Allow these as
+-pf and -pt, which are sent by libssh2.
 
 Debian Bug#919623
 ---
- util.c | 46 --
- 1 file changed, 44 insertions(+), 2 deletions(-)
+ util.c | 44 ++--
+ 1 file changed, 42 insertions(+), 2 deletions(-)
 
 diff --git a/util.c b/util.c
-index dc8c8fb..60b8ff6 100644
+index dc8c8fb..4203eac 100644
 --- a/util.c
 +++ b/util.c
-@@ -266,6 +266,45 @@ static int rsync_okay( char **vec )
+@@ -266,6

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Chris Lamb]

Hi Russ,

> I've not done an LTS security upload before, but it looks from the wiki
> that it uses the same security-master process as stable security updates.
> Please let me know if that's wrong.

This is mostly correct, yep! I made the following the changes to
your jessie diff:

  -  * The fix for the scp security vulneraability in 2.3.4-5+deb9u1
  +  * The fix for the scp security vulnerability in 2.3.4-4+deb8u2

.. and released this as a DLA-1660-2 "regression" update. I will leave
the stable update to the security team.


Best wishes,

-- 
  ,''`.
 : :'  : Chris Lamb
 `. `'`  la...@debian.org 🍥 chris-lamb.co.uk
   `-

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Roman Medina-Heigl Hernandez]

El 19/02/2019 a las 4:16, Russ Allbery escribió:

> Unfortunately, I took a closer look, and it turns out that this command
> was never safe.  It also allows arbitrary code excution on the server
> side if the client can write to $HOME.  This is because:
>
>--config=FILE
>   This specifies an alternate config file than the default.  This
>   is only relevant when --daemon is specified.  The default is
>   /etc/rsyncd.conf unless the daemon is running over a remote
>   shell program and the remote user is not the super-user; in that
>   case the default is rsyncd.conf in the current directory
>   (typically $HOME).
>
> That behavior of loading rsyncd.conf from the current directory was the
> piece I had missed before.

Well, in my case I had the following setting in rsyncd.conf:

path = /backup/synology

where path points to a different directory which is NOT $home nor
doesn't permit to reach $home.

So you cannot overwrite /home/synology/rsyncd.conf.

Well... provided that "path" config is not affected by path traversal
("../../home/synology"). At least, what I understand from rsyncd.conf
manual is that it shouldn't be possible:

**path**
This parameter specifies the directory in the daemon's filesystem to
make available in this module. You must specify this parameter for
each module in |rsyncd.conf|.


At the same time, my /etc/rssh.conf  has:

user=synology:022:10:  # rsync, with no chroot

So rssh shouldn't permit to use scp or other methods in order to
overwrite rsyncd.conf.


My setup seems safe (am I missing something?).


> Presumably, this is exactly the behavior that Synology relies on, but this
> means that if the client can write to this configuration file, it can just
> include a pre-xfer exec setting in that rsyncd.conf file and run commands
> on the server side.

Not in my setup.


> So, unfortunately we won't be able to fix Synology in a stable update,
> since it was relying on insecure behavior.  I'll continue with an update
> to fix the libssh2 regression.

I don't know how many people is using rssh (I guess not that many), but
if we limit its use more, it will probably die (it will probably die in
either case :-)).

Cheers,

-Román

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Russ Allbery]

Roman Medina-Heigl Hernandez  writes:

> Well, in my case I had the following setting in rsyncd.conf:

> path = /backup/synology

> where path points to a different directory which is NOT $home nor
> doesn't permit to reach $home.

> So you cannot overwrite /home/synology/rsyncd.conf.

Can the client just do:

rsync rsyncd.conf :./

first?

I think to make this safe the home directory has to not be owned by the
rssh user and not be writable by it.  That might be safe as long as the
current working directory of rsync is always the home directory.

(In your particular case, as mentioned in the previous message, I'm pretty
sure command="rsync --server -daemon ." in the authorized_keys file does
what you want since you don't need to allow other arbitrary rsync
commands.)

-- 
Russ Allbery (r...@debian.org)   

Re: rssh security update breaks rsync via Synology's "hyper backup"

[Roman Medina-Heigl Hernandez]

El 19/02/2019 a las 17:44, Russ Allbery escribió:

> Roman Medina-Heigl Hernandez  writes:
>
> So you cannot overwrite /home/synology/rsyncd.conf.
> Can the client just do:
>
> rsync rsyncd.conf :./
>
You're right, I was wrong. It's game over :)

> I think to make this safe the home directory has to not be owned by the
> rssh user and not be writable by it.  That might be safe as long as the
> current working directory of rsync is always the home directory.
> (In your particular case, as mentioned in the previous message, I'm pretty
> sure command="rsync --server -daemon ." in the authorized_keys file does
> what you want since you don't need to allow other arbitrary rsync
> commands.)

Thanks Russ and all for your help.

Cheers,

-Román