You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2017/08/18 17:33:14 UTC
Re: svn commit: r1805398 - in /subversion/site/publish/security:
CVE-2017-9800-advisory.txt CVE-2017-9800-advisory.txt.asc
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 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.