You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2017/08/18 10:33:10 UTC

svn commit: r1805398 - in /subversion/site/publish/security: CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc

Author: stsp
Date: Fri Aug 18 10:33:10 2017
New Revision: 1805398

URL: http://svn.apache.org/viewvc?rev=1805398&view=rev
Log:
* site/publish/security/CVE-2017-9800/advisory.txt,
  site/publish/security/CVE-2017-9800/advisory.txt.asc:
  Update the CVE-2017-9800 advisory.

  Fill in the Details section to explain how the attack actually works.

  Provide an example config setting which disables svn+ssh URLs and
  prevents the attack.
  Explain why the -- workaround cannot work with PuTTY.

  Move the discussion of "custom tunnels" further down to draw less
  attention to it. Most people don't use costom tunnels, and prominent
  mention of them will confuse users who are unfamiliar with the svn+ssh
  and tunnel mechanisms in the first place.

Modified:
    subversion/site/publish/security/CVE-2017-9800-advisory.txt
    subversion/site/publish/security/CVE-2017-9800-advisory.txt.asc

Modified: subversion/site/publish/security/CVE-2017-9800-advisory.txt
URL: http://svn.apache.org/viewvc/subversion/site/publish/security/CVE-2017-9800-advisory.txt?rev=1805398&r1=1805397&r2=1805398&view=diff
==============================================================================
--- subversion/site/publish/security/CVE-2017-9800-advisory.txt (original)
+++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 10:33:10 2017
@@ -53,7 +53,23 @@ Known fixed:
 Details:
 ========
 
-  (see "Summary:" above)
+  OpenSSH implements a ProxyCommand feature which instructs the client to
+  run an additional local command before opening a connection to the server.
+  The intention is that this local command will run a proxy server for the
+  connection to the SSH server.
+
+  This feature can be enabled on the command line with the -oProxyCommand
+  option switch. The -oProxyCommand option takes an arbitrary command as
+  its argument which will be executed by the ssh client before connecting
+  to the SSH server.
+
+  The attack makes use of this feature by placing a ProxyCommand option
+  with an arbitrary command into an svn+ssh:// URL.
+  A vulnerable svn client will pass this option to the ssh command, which
+  in turn will execute the arbitrary command provided by the attacker.
+
+  PuTTY's plink SSH client implements the same feature with a slightly
+  different option name "-proxycmd".
 
 Severity:
 =========
@@ -80,24 +96,36 @@ Recommendations:
   the included patch.
 
   3. Clients that are not able to execute the 'ssh' command are not vulnerable.
-  Note, however, that the name of the command may be changed by setting the
+  The name of the ssh command which is executed may be changed by setting the
   $SVN_SSH environment variable or by setting a value for the 'ssh' key
   in the "[tunnels]" section of the file "config" in the runtime configuration
-  area [1].  Moreover, the "[tunnels]" section may define additional tunnels;
-  those may be vulnerable if they do not perform input validation on their
-  first argument, which contains the hostname to connect to.
-
-  By default, only the "ssh" tunnel is available.  It is available even if it
-  is commented out in the file "config".  The default definition of the "ssh"
-  tunnel is equivalent to:
+  area [1].
+  By default, only the "ssh" tunnel is configured.  It is available even if
+  it is commented out in the file "config".  The default definition of the
+  "ssh" tunnel is equivalent to:
       ssh = $SVN_SSH ssh -q
-  If that line is not commented out and not set to the default, audit that line
-  as explained below for additional/custom tunnels.  If that line is commented
-  out or set to the default, two different fixes are available: either
-  uncomment that line and change the setting to
+  If the value of this option is set to a non-existent path, then svn+ssh://
+  URLs will no longer work but the svn client will not be vulnerable:
+      ssh = /this/path/does/not/exist
+
+  However, if OpenSSH is used as SSH client (the default on most UNIX-like
+  systems), then svn+ssh:// URLs can still be used safely. Either change the
+  configuration file setting to:
       ssh = $SVN_SSH ssh -q --
-  or set the environment variable "SVN_SSH" to the value "ssh -q --".
+  or set the environment variable "SVN_SSH" to the value:
+     ssh -q --
+  The -- argument tells OpenSSH to stop processing subsequent arguments as
+  command line options, and hence neuters the attack because a -oProxyCommand
+  option on the command line will no longer be evaluated as an option.
+  If PuTTY is used as SSH client (the default on Windows systems, including
+  TortoiseSVN) this trick WILL NOT WORK because PuTTY evaluates command
+  line options even after -- occurs on its command line. If using PuTTY,
+  either the svn client must be upgraded or svn+ssh:// URLs must be disabled
+  entirely as described above.
 
+  The "[tunnels]" section may define additional third-party custom tunnels;
+  those may be vulnerable if they do not perform input validation on their
+  first argument, which contains the hostname to connect to.
   Custom tunnels are invoked with three arguments: the hostname to connect to,
   the string "svnserve" and the string "-t".  It is recommended that custom
   tunnel definitions be audited for correct handling of unusual or invalid

Modified: subversion/site/publish/security/CVE-2017-9800-advisory.txt.asc
URL: http://svn.apache.org/viewvc/subversion/site/publish/security/CVE-2017-9800-advisory.txt.asc?rev=1805398&r1=1805397&r2=1805398&view=diff
==============================================================================
--- subversion/site/publish/security/CVE-2017-9800-advisory.txt.asc (original)
+++ subversion/site/publish/security/CVE-2017-9800-advisory.txt.asc Fri Aug 18 10:33:10 2017
@@ -1,16 +1,10 @@
 -----BEGIN PGP SIGNATURE-----
 
-iQIrBAABCAAdFiEEbrYLY3zlrL8kSaLa2yfpl0Ka8gwFAlmM5HEACgkQ2yfpl0Ka
-8gwSMA+/cMyyyhZL4gzKlLaoXaV7exWMOQ43gLSyxYlQu6EJFUKOBrqboWw9A2R/
-2PLloCAIl5mMsQyNCrCxN8HNywRLnZaSLLx0dUTinoiuiqZVTriLKKLHpdRYpLD4
-lNdUzs1EBEMaN0QCHpoXxIEzgHOt71KKocyRUwvKlX/QctSIt+pkALEF5a8LGFX3
-O3KMcpckT2JwMTjK01xxbLl2xwLQ84k0I7zDBm/QTkWAsxaXVBQT9tjJSnqtvEA1
-n4rryC1E33gmHngs9yPZTN0HlNRdMrDcR5hQR9l3qvrRA7rYLA23Vvn/prFfq1+A
-BhYKKUgqRUEqCY2LVA/tqs8CnkIiMEFGCO88UKjn33AH+fDy/W31mipvD5QOo0wh
-YJDpBKseQjDyc+zAgt2OMFybweAFBj3qpX5yvvrSG+fGHuiSdp66pDviPKcjVTOt
-lkDWB2m5nr21f/0mGw5GHFCAMLZBO/+X1iNlCK78FUbYfX7RDNRF4kDCl3wmX9VT
-mDmKLdWMwwpIaA5w965w4n8edsJgsMsph5TDnzUCFSk+tLprplAK0MiDBJWhscuk
-3nUNwXIhEr5AHqoRE2yutqV/PtctgS+hCAV4UHTkJCo52P7y1OIevt3mbVM08vAx
-WNQOQZP83MD8QAFOyR9Lv4wJQIp/YGy9U0kqrlcx
-=Sfuf
+iQEcBAABAgAGBQJZlsHzAAoJEE99uqmaWblzF8wIAKY9wfXmQiQLoNYoIUir/QF8
+tYl2ielOyOmRDVCWO3+U5hy93vTVPfPX+EB97uqTyVwstoffjK4kGHP2eru+d540
+9WD/F2mJvY/uPivX+49/u4veJArQXoQmYOD42ajO5LUluol/TtLkaaw2kfDDPy3F
+JRj4R3DSlcuvYzvoK9OJmNrwqPKMDm9g5/cjA/+526NGed2NIhC2cfka10D9VlUI
+OntzRB9OkzYR8PFViaL5RJXHsqlvQgee1o/yvWVUIfVml9g4ChZwlQsruuDBdDj1
+26mi96DsnX/zmGy56FPw2o0tH7wLqHY74T44zA6ylSghe/2kxijQ9n82YucN4Gg=
+=1lp1
 -----END PGP SIGNATURE-----



Re: svn commit: r1805398 - in /subversion/site/publish/security: CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc

Posted by Stefan Sperling <st...@apache.org>.
On Fri, Aug 18, 2017 at 05:33:14PM +0000, Daniel Shahaf wrote:
> stsp@apache.org wrote on Fri, 18 Aug 2017 10:33 +0000:
> > +++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 10:33:10 2017
> > @@ -80,24 +96,36 @@ Recommendations:
> >    the included patch.
> >  
> >    3. Clients that are not able to execute the 'ssh' command are not vulnerable.
> >    ⋮
> > +  If the value of this option is set to a non-existent path, then svn+ssh://
> > +  URLs will no longer work but the svn client will not be vulnerable:
> > +      ssh = /this/path/does/not/exist
> 
> This workaround does not work if --config-option is passed on the command line.
> 
> I think it is not unusual to invoke svn(1) through a wrapper that always sets
> some options.  (In this specific case, someone might have an alias to svn that
> sets config:tunnels:ssh differently depending on what jumphost they need to use.)
> 
> I think the advisory should be complete, i.e., cover every supported use-case,
> so it can be used as a checklist that users go through, and once they have done
> so they can be sure that they are no longer vulnerable.  That said, I do agree
> that a shortened version that covers only the most common use-cases would be a
> Good Thing as well.
> 
> Here's suggested text for the former; the latter can be addressed in follow-up
> commits:
> 
> Index: CVE-2017-9800-advisory.txt
> ===================================================================
> --- CVE-2017-9800-advisory.txt	(revision 1805448)
> +++ CVE-2017-9800-advisory.txt	(working copy)
> @@ -107,6 +107,9 @@
>    If the value of this option is set to a non-existent path, then svn+ssh://
>    URLs will no longer work but the svn client will not be vulnerable:
>        ssh = /this/path/does/not/exist
> +  This trick WILL NOT WORK if the option setting is overridden by the
> +  --config-option=config:tunnels:ssh=foo command-line option of the
> +  command-line client.

Is the same not true for the 'ssh -q --' trick?

I would suggest we add a paragraph such as:

Index: CVE-2017-9800-advisory.txt
===================================================================
--- CVE-2017-9800-advisory.txt	(revision 1805402)
+++ CVE-2017-9800-advisory.txt	(working copy)
@@ -123,6 +123,11 @@ Recommendations:
   either the svn client must be upgraded or svn+ssh:// URLs must be disabled
   entirely as described above.
 
+  Note that the svn client supports the --config-option option which can
+  be used to override settings specified in the configuration file.
+  If this option is used to configure the ssh tunnel command, the same
+  guidelines should be applied to prevent an attack.
+
   The "[tunnels]" section may define additional third-party custom tunnels;
   those may be vulnerable if they do not perform input validation on their
   first argument, which contains the hostname to connect to.

Re: svn commit: r1805398 - in /subversion/site/publish/security: CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc

Posted by Stefan Sperling <st...@apache.org>.
On Fri, Aug 18, 2017 at 05:33:14PM +0000, Daniel Shahaf wrote:
> stsp@apache.org wrote on Fri, 18 Aug 2017 10:33 +0000:
> > +++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 10:33:10 2017
> > @@ -80,24 +96,36 @@ Recommendations:
> >    the included patch.
> >  
> >    3. Clients that are not able to execute the 'ssh' command are not vulnerable.
> >    ⋮
> > +  If the value of this option is set to a non-existent path, then svn+ssh://
> > +  URLs will no longer work but the svn client will not be vulnerable:
> > +      ssh = /this/path/does/not/exist
> 
> This workaround does not work if --config-option is passed on the command line.
> 
> I think it is not unusual to invoke svn(1) through a wrapper that always sets
> some options.  (In this specific case, someone might have an alias to svn that
> sets config:tunnels:ssh differently depending on what jumphost they need to use.)
> 
> I think the advisory should be complete, i.e., cover every supported use-case,
> so it can be used as a checklist that users go through, and once they have done
> so they can be sure that they are no longer vulnerable.  That said, I do agree
> that a shortened version that covers only the most common use-cases would be a
> Good Thing as well.
> 
> Here's suggested text for the former; the latter can be addressed in follow-up
> commits:
> 
> Index: CVE-2017-9800-advisory.txt
> ===================================================================
> --- CVE-2017-9800-advisory.txt	(revision 1805448)
> +++ CVE-2017-9800-advisory.txt	(working copy)
> @@ -107,6 +107,9 @@
>    If the value of this option is set to a non-existent path, then svn+ssh://
>    URLs will no longer work but the svn client will not be vulnerable:
>        ssh = /this/path/does/not/exist
> +  This trick WILL NOT WORK if the option setting is overridden by the
> +  --config-option=config:tunnels:ssh=foo command-line option of the
> +  command-line client.

Is the same not true for the 'ssh -q --' trick?

I would suggest we add a paragraph such as:

Index: CVE-2017-9800-advisory.txt
===================================================================
--- CVE-2017-9800-advisory.txt	(revision 1805402)
+++ CVE-2017-9800-advisory.txt	(working copy)
@@ -123,6 +123,11 @@ Recommendations:
   either the svn client must be upgraded or svn+ssh:// URLs must be disabled
   entirely as described above.
 
+  Note that the svn client supports the --config-option option which can
+  be used to override settings specified in the configuration file.
+  If this option is used to configure the ssh tunnel command, the same
+  guidelines should be applied to prevent an attack.
+
   The "[tunnels]" section may define additional third-party custom tunnels;
   those may be vulnerable if they do not perform input validation on their
   first argument, which contains the hostname to connect to.

Re: svn commit: r1805398 - in /subversion/site/publish/security: CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Fri, 18 Aug 2017 10:33 +0000:
> +++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 10:33:10 2017
> @@ -80,24 +96,36 @@ Recommendations:
>    the included patch.
>  
>    3. Clients that are not able to execute the 'ssh' command are not vulnerable.
>    ⋮
> +  If the value of this option is set to a non-existent path, then svn+ssh://
> +  URLs will no longer work but the svn client will not be vulnerable:
> +      ssh = /this/path/does/not/exist

This workaround does not work if --config-option is passed on the command line.

I think it is not unusual to invoke svn(1) through a wrapper that always sets
some options.  (In this specific case, someone might have an alias to svn that
sets config:tunnels:ssh differently depending on what jumphost they need to use.)

I think the advisory should be complete, i.e., cover every supported use-case,
so it can be used as a checklist that users go through, and once they have done
so they can be sure that they are no longer vulnerable.  That said, I do agree
that a shortened version that covers only the most common use-cases would be a
Good Thing as well.

Here's suggested text for the former; the latter can be addressed in follow-up
commits:

Index: CVE-2017-9800-advisory.txt
===================================================================
--- CVE-2017-9800-advisory.txt	(revision 1805448)
+++ CVE-2017-9800-advisory.txt	(working copy)
@@ -107,6 +107,9 @@
   If the value of this option is set to a non-existent path, then svn+ssh://
   URLs will no longer work but the svn client will not be vulnerable:
       ssh = /this/path/does/not/exist
+  This trick WILL NOT WORK if the option setting is overridden by the
+  --config-option=config:tunnels:ssh=foo command-line option of the
+  command-line client.
 
   However, if OpenSSH is used as SSH client (the default on most UNIX-like
   systems), then svn+ssh:// URLs can still be used safely. Either change the
Index: CVE-2017-9800-advisory.txt.asc
===================================================================
--- CVE-2017-9800-advisory.txt.asc	(revision 1805448)
+++ CVE-2017-9800-advisory.txt.asc	(working copy)
@@ -1,4 +1,5 @@
 -----BEGIN PGP SIGNATURE-----
+Comment: TODO: re-sign
 
 iQEcBAABAgAGBQJZlsHzAAoJEE99uqmaWblzF8wIAKY9wfXmQiQLoNYoIUir/QF8
 tYl2ielOyOmRDVCWO3+U5hy93vTVPfPX+EB97uqTyVwstoffjK4kGHP2eru+d540

Re: svn commit: r1805398 - in /subversion/site/publish/security: CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stsp@apache.org wrote on Fri, 18 Aug 2017 10:33 +0000:
> +++ subversion/site/publish/security/CVE-2017-9800-advisory.txt Fri Aug 18 10:33:10 2017
> @@ -80,24 +96,36 @@ Recommendations:
>    the included patch.
>  
>    3. Clients that are not able to execute the 'ssh' command are not vulnerable.
>    ⋮
> +  If the value of this option is set to a non-existent path, then svn+ssh://
> +  URLs will no longer work but the svn client will not be vulnerable:
> +      ssh = /this/path/does/not/exist

This workaround does not work if --config-option is passed on the command line.

I think it is not unusual to invoke svn(1) through a wrapper that always sets
some options.  (In this specific case, someone might have an alias to svn that
sets config:tunnels:ssh differently depending on what jumphost they need to use.)

I think the advisory should be complete, i.e., cover every supported use-case,
so it can be used as a checklist that users go through, and once they have done
so they can be sure that they are no longer vulnerable.  That said, I do agree
that a shortened version that covers only the most common use-cases would be a
Good Thing as well.

Here's suggested text for the former; the latter can be addressed in follow-up
commits:

Index: CVE-2017-9800-advisory.txt
===================================================================
--- CVE-2017-9800-advisory.txt	(revision 1805448)
+++ CVE-2017-9800-advisory.txt	(working copy)
@@ -107,6 +107,9 @@
   If the value of this option is set to a non-existent path, then svn+ssh://
   URLs will no longer work but the svn client will not be vulnerable:
       ssh = /this/path/does/not/exist
+  This trick WILL NOT WORK if the option setting is overridden by the
+  --config-option=config:tunnels:ssh=foo command-line option of the
+  command-line client.
 
   However, if OpenSSH is used as SSH client (the default on most UNIX-like
   systems), then svn+ssh:// URLs can still be used safely. Either change the
Index: CVE-2017-9800-advisory.txt.asc
===================================================================
--- CVE-2017-9800-advisory.txt.asc	(revision 1805448)
+++ CVE-2017-9800-advisory.txt.asc	(working copy)
@@ -1,4 +1,5 @@
 -----BEGIN PGP SIGNATURE-----
+Comment: TODO: re-sign
 
 iQEcBAABAgAGBQJZlsHzAAoJEE99uqmaWblzF8wIAKY9wfXmQiQLoNYoIUir/QF8
 tYl2ielOyOmRDVCWO3+U5hy93vTVPfPX+EB97uqTyVwstoffjK4kGHP2eru+d540