Patchwork [2,of,2,STABLE] tests-subrepo-git: emit a different "pwned" message based on the test

login
register
mail settings
Submitter Danek Duvall
Date May 27, 2016, 10:32 p.m.
Message ID <79f6cd752245a56bf641.1464388364@smelly.us.oracle.com>
Download mbox | patch
Permalink /patch/15226/
State Superseded
Commit a9764ab80e11bcf6a37255db7dd079011f767c6c
Headers show

Comments

Danek Duvall - May 27, 2016, 10:32 p.m.
# HG changeset patch
# User Danek Duvall <danek.duvall@oracle.com>
# Date 1464387603 25200
#      Fri May 27 15:20:03 2016 -0700
# Branch stable
# Node ID 79f6cd752245a56bf6418f7535eb589260351f2e
# Parent  c2c56e205a51017b4a2a205efab75882b9f86099
tests-subrepo-git: emit a different "pwned" message based on the test

Having a single "pwned" message which may or may not be emitted during the
tests for CVE-2016-3068 leads to extra confusion.  Allow each test to emit
a more detailed message based on what the expectations are.

In both cases, we expect a version of git which has had the vulnerability
plugged, as well as a version of mercurial which also knows about
GIT_ALLOW_PROTOCOL.  For the first test, we make sure GIT_ALLOW_PROTOCOL is
unset, meaning that the ext-protocol subrepo should be ignored; if it
isn't, there's either a problem with mercurial or the installed copy of
git.

For the second test, we explicitly allow ext-protocol subrepos, which means
that the subrepo will be accessed and a message emitted confirming that
this was, in fact, our intention.
Augie Fackler - May 29, 2016, 9:13 p.m.
On Fri, May 27, 2016 at 03:32:44PM -0700, danek.duvall@oracle.com wrote:
> # HG changeset patch
> # User Danek Duvall <danek.duvall@oracle.com>
> # Date 1464387603 25200
> #      Fri May 27 15:20:03 2016 -0700
> # Branch stable
> # Node ID 79f6cd752245a56bf6418f7535eb589260351f2e
> # Parent  c2c56e205a51017b4a2a205efab75882b9f86099
> tests-subrepo-git: emit a different "pwned" message based on the test

Yes! Queued with delight.

>
> Having a single "pwned" message which may or may not be emitted during the
> tests for CVE-2016-3068 leads to extra confusion.  Allow each test to emit
> a more detailed message based on what the expectations are.
>
> In both cases, we expect a version of git which has had the vulnerability
> plugged, as well as a version of mercurial which also knows about
> GIT_ALLOW_PROTOCOL.  For the first test, we make sure GIT_ALLOW_PROTOCOL is
> unset, meaning that the ext-protocol subrepo should be ignored; if it
> isn't, there's either a problem with mercurial or the installed copy of
> git.
>
> For the second test, we explicitly allow ext-protocol subrepos, which means
> that the subrepo will be accessed and a message emitted confirming that
> this was, in fact, our intention.
>
> diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
> --- a/tests/test-subrepo-git.t
> +++ b/tests/test-subrepo-git.t
> @@ -1135,7 +1135,7 @@ make sure we show changed files, rather
>  test for Git CVE-2016-3068
>    $ hg init malicious-subrepository
>    $ cd malicious-subrepository
> -  $ echo "s = [git]ext::sh -c echo% pwned% >pwned.txt" > .hgsub
> +  $ echo "s = [git]ext::sh -c echo% \$PWNED_MSG% >pwned.txt" > .hgsub
>    $ git init s
>    Initialized empty Git repository in $TESTTMP/tc/malicious-subrepository/s/.git/
>    $ cd s
> @@ -1146,26 +1146,29 @@ test for Git CVE-2016-3068
>    $ hg commit -m "add subrepo"
>    $ cd ..
>    $ rm -f pwned.txt
> -  $ env -u GIT_ALLOW_PROTOCOL hg clone malicious-subrepository malicious-subrepository-protected
> +  $ env -u GIT_ALLOW_PROTOCOL \
> +  > PWNED_MSG="your git is too old or mercurial has regressed" hg clone \
> +  > malicious-subrepository malicious-subrepository-protected
>    Cloning into '$TESTTMP/tc/malicious-subrepository-protected/s'... (glob)
>    fatal: transport 'ext' not allowed
>    updating to branch default
> -  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
> +  cloning subrepo s from ext::sh -c echo% $PWNED_MSG% >pwned.txt
>    abort: git clone error 128 in s (in subrepo s)
>    [255]
>    $ test -f pwned.txt && cat pwned.txt || true
>
>  whitelisting of ext should be respected (that's the git submodule behaviour)
>    $ rm -f pwned.txt
> -  $ env GIT_ALLOW_PROTOCOL=ext hg clone malicious-subrepository malicious-subrepository-clone-allowed
> +  $ env GIT_ALLOW_PROTOCOL=ext PWNED_MSG="you asked for it" hg clone \
> +  > malicious-subrepository malicious-subrepository-clone-allowed
>    Cloning into '$TESTTMP/tc/malicious-subrepository-clone-allowed/s'... (glob)
>    fatal: Could not read from remote repository.
>
>    Please make sure you have the correct access rights
>    and the repository exists.
>    updating to branch default
> -  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
> +  cloning subrepo s from ext::sh -c echo% $PWNED_MSG% >pwned.txt
>    abort: git clone error 128 in s (in subrepo s)
>    [255]
>    $ cat pwned.txt
> -  pwned
> +  you asked for it
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - May 30, 2016, 1:58 p.m.
On Fri, 27 May 2016 15:32:44 -0700, danek.duvall@oracle.com wrote:
> # HG changeset patch
> # User Danek Duvall <danek.duvall@oracle.com>
> # Date 1464387603 25200
> #      Fri May 27 15:20:03 2016 -0700
> # Branch stable
> # Node ID 79f6cd752245a56bf6418f7535eb589260351f2e
> # Parent  c2c56e205a51017b4a2a205efab75882b9f86099
> tests-subrepo-git: emit a different "pwned" message based on the test

> -  $ echo "s = [git]ext::sh -c echo% pwned% >pwned.txt" > .hgsub
> +  $ echo "s = [git]ext::sh -c echo% \$PWNED_MSG% >pwned.txt" > .hgsub

Maybe I'm paranoid, but we'd be better to print static message as well
as $PWNED_MSG in case we had typo like PWNDE_MSG.

Patch

diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
--- a/tests/test-subrepo-git.t
+++ b/tests/test-subrepo-git.t
@@ -1135,7 +1135,7 @@  make sure we show changed files, rather 
 test for Git CVE-2016-3068
   $ hg init malicious-subrepository
   $ cd malicious-subrepository
-  $ echo "s = [git]ext::sh -c echo% pwned% >pwned.txt" > .hgsub
+  $ echo "s = [git]ext::sh -c echo% \$PWNED_MSG% >pwned.txt" > .hgsub
   $ git init s
   Initialized empty Git repository in $TESTTMP/tc/malicious-subrepository/s/.git/
   $ cd s
@@ -1146,26 +1146,29 @@  test for Git CVE-2016-3068
   $ hg commit -m "add subrepo"
   $ cd ..
   $ rm -f pwned.txt
-  $ env -u GIT_ALLOW_PROTOCOL hg clone malicious-subrepository malicious-subrepository-protected
+  $ env -u GIT_ALLOW_PROTOCOL \
+  > PWNED_MSG="your git is too old or mercurial has regressed" hg clone \
+  > malicious-subrepository malicious-subrepository-protected
   Cloning into '$TESTTMP/tc/malicious-subrepository-protected/s'... (glob)
   fatal: transport 'ext' not allowed
   updating to branch default
-  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
+  cloning subrepo s from ext::sh -c echo% $PWNED_MSG% >pwned.txt
   abort: git clone error 128 in s (in subrepo s)
   [255]
   $ test -f pwned.txt && cat pwned.txt || true
 
 whitelisting of ext should be respected (that's the git submodule behaviour)
   $ rm -f pwned.txt
-  $ env GIT_ALLOW_PROTOCOL=ext hg clone malicious-subrepository malicious-subrepository-clone-allowed
+  $ env GIT_ALLOW_PROTOCOL=ext PWNED_MSG="you asked for it" hg clone \
+  > malicious-subrepository malicious-subrepository-clone-allowed
   Cloning into '$TESTTMP/tc/malicious-subrepository-clone-allowed/s'... (glob)
   fatal: Could not read from remote repository.
   
   Please make sure you have the correct access rights
   and the repository exists.
   updating to branch default
-  cloning subrepo s from ext::sh -c echo% pwned% >pwned.txt
+  cloning subrepo s from ext::sh -c echo% $PWNED_MSG% >pwned.txt
   abort: git clone error 128 in s (in subrepo s)
   [255]
   $ cat pwned.txt
-  pwned
+  you asked for it