From f7bcd554fae642585af5f99c3c858eb2d343e1da Mon Sep 17 00:00:00 2001 From: Tommi Virtanen Date: Wed, 19 Mar 2008 21:28:46 +0200 Subject: Test that incoming paths cannot contain /../ --- gitosis/test/test_serve.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'gitosis/test/test_serve.py') diff --git a/gitosis/test/test_serve.py b/gitosis/test/test_serve.py index d6030d2..cf54cc6 100644 --- a/gitosis/test/test_serve.py +++ b/gitosis/test/test_serve.py @@ -57,6 +57,18 @@ def test_bad_unsafeArguments(): eq(str(e), 'Arguments to command look dangerous') assert isinstance(e, serve.ServingError) +def test_bad_unsafeArguments_dotdot(): + cfg = RawConfigParser() + e = assert_raises( + serve.UnsafeArgumentsError, + serve.serve, + cfg=cfg, + user='jdoe', + command='git-upload-pack something/../evil', + ) + eq(str(e), 'Arguments to command look dangerous') + assert isinstance(e, serve.ServingError) + def test_bad_forbiddenCommand_read(): cfg = RawConfigParser() e = assert_raises( -- cgit v1.2.3 From f839f889b607c9920659516959795859aab0a86e Mon Sep 17 00:00:00 2001 From: Tommi Virtanen Date: Wed, 19 Mar 2008 21:52:03 +0200 Subject: Make serve acceptable path unit tests more careful. Tests used to trigger the wanted security exception merely by being unquoted, that's not good enough. --- gitosis/test/test_serve.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) (limited to 'gitosis/test/test_serve.py') diff --git a/gitosis/test/test_serve.py b/gitosis/test/test_serve.py index cf54cc6..23b6a6f 100644 --- a/gitosis/test/test_serve.py +++ b/gitosis/test/test_serve.py @@ -45,14 +45,38 @@ def test_bad_command(): eq(str(e), 'Unknown command denied') assert isinstance(e, serve.ServingError) -def test_bad_unsafeArguments(): +def test_bad_unsafeArguments_notQuoted(): cfg = RawConfigParser() e = assert_raises( serve.UnsafeArgumentsError, serve.serve, cfg=cfg, user='jdoe', - command='git-upload-pack /evil/attack', + command="git-upload-pack foo", + ) + eq(str(e), 'Arguments to command look dangerous') + assert isinstance(e, serve.ServingError) + +def test_bad_unsafeArguments_absolute(): + cfg = RawConfigParser() + e = assert_raises( + serve.UnsafeArgumentsError, + serve.serve, + cfg=cfg, + user='jdoe', + command="git-upload-pack '/evil/attack'", + ) + eq(str(e), 'Arguments to command look dangerous') + assert isinstance(e, serve.ServingError) + +def test_bad_unsafeArguments_badCharacters(): + cfg = RawConfigParser() + e = assert_raises( + serve.UnsafeArgumentsError, + serve.serve, + cfg=cfg, + user='jdoe', + command="git-upload-pack 'ev!l'", ) eq(str(e), 'Arguments to command look dangerous') assert isinstance(e, serve.ServingError) @@ -64,7 +88,7 @@ def test_bad_unsafeArguments_dotdot(): serve.serve, cfg=cfg, user='jdoe', - command='git-upload-pack something/../evil', + command="git-upload-pack 'something/../evil'", ) eq(str(e), 'Arguments to command look dangerous') assert isinstance(e, serve.ServingError) -- cgit v1.2.3 From 4d8ba7788d10e62928404b0272de241580e00e92 Mon Sep 17 00:00:00 2001 From: Tommi Virtanen Date: Wed, 19 Mar 2008 21:49:47 +0200 Subject: Allow absolute paths in repo paths, treat them as relative. As the only convenient way to use non-standard SSH ports with git is via the ssh://user@host:port/path syntax, and that syntax forces absolute urls, just force convert absolute paths to relative paths; you'll never really want absolute paths via gitosis, anyway. --- gitosis/test/test_serve.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'gitosis/test/test_serve.py') diff --git a/gitosis/test/test_serve.py b/gitosis/test/test_serve.py index 23b6a6f..a223c43 100644 --- a/gitosis/test/test_serve.py +++ b/gitosis/test/test_serve.py @@ -57,18 +57,6 @@ def test_bad_unsafeArguments_notQuoted(): eq(str(e), 'Arguments to command look dangerous') assert isinstance(e, serve.ServingError) -def test_bad_unsafeArguments_absolute(): - cfg = RawConfigParser() - e = assert_raises( - serve.UnsafeArgumentsError, - serve.serve, - cfg=cfg, - user='jdoe', - command="git-upload-pack '/evil/attack'", - ) - eq(str(e), 'Arguments to command look dangerous') - assert isinstance(e, serve.ServingError) - def test_bad_unsafeArguments_badCharacters(): cfg = RawConfigParser() e = assert_raises( @@ -402,3 +390,23 @@ def test_push_inits_sets_export_ok(): path = os.path.join(repositories, 'foo.git', 'git-daemon-export-ok') assert os.path.exists(path) +def test_absolute(): + # as the only convenient way to use non-standard SSH ports with + # git is via the ssh://user@host:port/path syntax, and that syntax + # forces absolute urls, just force convert absolute paths to + # relative paths; you'll never really want absolute paths via + # gitosis, anyway. + tmp = util.maketemp() + repository.init(os.path.join(tmp, 'foo.git')) + cfg = RawConfigParser() + cfg.add_section('gitosis') + cfg.set('gitosis', 'repositories', tmp) + cfg.add_section('group foo') + cfg.set('group foo', 'members', 'jdoe') + cfg.set('group foo', 'readonly', 'foo') + got = serve.serve( + cfg=cfg, + user='jdoe', + command="git-upload-pack '/foo'", + ) + eq(got, "git-upload-pack '%s/foo.git'" % tmp) -- cgit v1.2.3 From 38561aa6a51a2ef6cc04aa119481df62d213ffa4 Mon Sep 17 00:00:00 2001 From: Tommi Virtanen Date: Sat, 19 Apr 2008 19:10:36 +0300 Subject: Understand the popular gitosis.conf typo "writeable". Log a warning still, don't want that to get too common. --- gitosis/test/test_serve.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'gitosis/test/test_serve.py') diff --git a/gitosis/test/test_serve.py b/gitosis/test/test_serve.py index a223c43..56d50b0 100644 --- a/gitosis/test/test_serve.py +++ b/gitosis/test/test_serve.py @@ -1,7 +1,9 @@ from nose.tools import eq_ as eq from gitosis.test.util import assert_raises +import logging import os +from cStringIO import StringIO from ConfigParser import RawConfigParser from gitosis import serve @@ -410,3 +412,32 @@ def test_absolute(): command="git-upload-pack '/foo'", ) eq(got, "git-upload-pack '%s/foo.git'" % tmp) + +def test_typo_writeable(): + tmp = util.maketemp() + repository.init(os.path.join(tmp, 'foo.git')) + cfg = RawConfigParser() + cfg.add_section('gitosis') + cfg.set('gitosis', 'repositories', tmp) + cfg.add_section('group foo') + cfg.set('group foo', 'members', 'jdoe') + cfg.set('group foo', 'writeable', 'foo') + log = logging.getLogger('gitosis.serve') + buf = StringIO() + handler = logging.StreamHandler(buf) + log.addHandler(handler) + try: + got = serve.serve( + cfg=cfg, + user='jdoe', + command="git-receive-pack 'foo'", + ) + finally: + log.removeHandler(handler) + eq(got, "git-receive-pack '%s/foo.git'" % tmp) + handler.flush() + eq( + buf.getvalue(), + "Repository 'foo' config has typo \"writeable\", shou" + +"ld be \"writable\"\n", + ) -- cgit v1.2.3 From 72c754b2f03a139122dc4a3877b05704fa88f751 Mon Sep 17 00:00:00 2001 From: Tommi Virtanen Date: Thu, 26 Jun 2008 11:33:48 +0300 Subject: Accept "git upload-pack" etc, for future compatibility. --- gitosis/test/test_serve.py | 100 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 6 deletions(-) (limited to 'gitosis/test/test_serve.py') diff --git a/gitosis/test/test_serve.py b/gitosis/test/test_serve.py index 56d50b0..f1c1930 100644 --- a/gitosis/test/test_serve.py +++ b/gitosis/test/test_serve.py @@ -23,7 +23,7 @@ def test_bad_newLine(): eq(str(e), 'Command may not contain newline') assert isinstance(e, serve.ServingError) -def test_bad_nospace(): +def test_bad_dash_noargs(): cfg = RawConfigParser() e = assert_raises( serve.UnknownCommandError, @@ -35,6 +35,18 @@ def test_bad_nospace(): eq(str(e), 'Unknown command denied') assert isinstance(e, serve.ServingError) +def test_bad_space_noargs(): + cfg = RawConfigParser() + e = assert_raises( + serve.UnknownCommandError, + serve.serve, + cfg=cfg, + user='jdoe', + command='git upload-pack', + ) + eq(str(e), 'Unknown command denied') + assert isinstance(e, serve.ServingError) + def test_bad_command(): cfg = RawConfigParser() e = assert_raises( @@ -83,7 +95,7 @@ def test_bad_unsafeArguments_dotdot(): eq(str(e), 'Arguments to command look dangerous') assert isinstance(e, serve.ServingError) -def test_bad_forbiddenCommand_read(): +def test_bad_forbiddenCommand_read_dash(): cfg = RawConfigParser() e = assert_raises( serve.ReadAccessDenied, @@ -96,7 +108,20 @@ def test_bad_forbiddenCommand_read(): assert isinstance(e, serve.AccessDenied) assert isinstance(e, serve.ServingError) -def test_bad_forbiddenCommand_write_noAccess(): +def test_bad_forbiddenCommand_read_space(): + cfg = RawConfigParser() + e = assert_raises( + serve.ReadAccessDenied, + serve.serve, + cfg=cfg, + user='jdoe', + command="git upload-pack 'foo'", + ) + eq(str(e), 'Repository read access denied') + assert isinstance(e, serve.AccessDenied) + assert isinstance(e, serve.ServingError) + +def test_bad_forbiddenCommand_write_noAccess_dash(): cfg = RawConfigParser() e = assert_raises( serve.ReadAccessDenied, @@ -111,7 +136,22 @@ def test_bad_forbiddenCommand_write_noAccess(): assert isinstance(e, serve.AccessDenied) assert isinstance(e, serve.ServingError) -def test_bad_forbiddenCommand_write_readAccess(): +def test_bad_forbiddenCommand_write_noAccess_space(): + cfg = RawConfigParser() + e = assert_raises( + serve.ReadAccessDenied, + serve.serve, + cfg=cfg, + user='jdoe', + command="git receive-pack 'foo'", + ) + # error message talks about read in an effort to make it more + # obvious that jdoe doesn't have *even* read access + eq(str(e), 'Repository read access denied') + assert isinstance(e, serve.AccessDenied) + assert isinstance(e, serve.ServingError) + +def test_bad_forbiddenCommand_write_readAccess_dash(): cfg = RawConfigParser() cfg.add_section('group foo') cfg.set('group foo', 'members', 'jdoe') @@ -127,7 +167,23 @@ def test_bad_forbiddenCommand_write_readAccess(): assert isinstance(e, serve.AccessDenied) assert isinstance(e, serve.ServingError) -def test_simple_read(): +def test_bad_forbiddenCommand_write_readAccess_space(): + cfg = RawConfigParser() + cfg.add_section('group foo') + cfg.set('group foo', 'members', 'jdoe') + cfg.set('group foo', 'readonly', 'foo') + e = assert_raises( + serve.WriteAccessDenied, + serve.serve, + cfg=cfg, + user='jdoe', + command="git receive-pack 'foo'", + ) + eq(str(e), 'Repository write access denied') + assert isinstance(e, serve.AccessDenied) + assert isinstance(e, serve.ServingError) + +def test_simple_read_dash(): tmp = util.maketemp() repository.init(os.path.join(tmp, 'foo.git')) cfg = RawConfigParser() @@ -143,7 +199,23 @@ def test_simple_read(): ) eq(got, "git-upload-pack '%s/foo.git'" % tmp) -def test_simple_write(): +def test_simple_read_space(): + tmp = util.maketemp() + repository.init(os.path.join(tmp, 'foo.git')) + cfg = RawConfigParser() + cfg.add_section('gitosis') + cfg.set('gitosis', 'repositories', tmp) + cfg.add_section('group foo') + cfg.set('group foo', 'members', 'jdoe') + cfg.set('group foo', 'readonly', 'foo') + got = serve.serve( + cfg=cfg, + user='jdoe', + command="git upload-pack 'foo'", + ) + eq(got, "git upload-pack '%s/foo.git'" % tmp) + +def test_simple_write_dash(): tmp = util.maketemp() repository.init(os.path.join(tmp, 'foo.git')) cfg = RawConfigParser() @@ -159,6 +231,22 @@ def test_simple_write(): ) eq(got, "git-receive-pack '%s/foo.git'" % tmp) +def test_simple_write_space(): + tmp = util.maketemp() + repository.init(os.path.join(tmp, 'foo.git')) + cfg = RawConfigParser() + cfg.add_section('gitosis') + cfg.set('gitosis', 'repositories', tmp) + cfg.add_section('group foo') + cfg.set('group foo', 'members', 'jdoe') + cfg.set('group foo', 'writable', 'foo') + got = serve.serve( + cfg=cfg, + user='jdoe', + command="git receive-pack 'foo'", + ) + eq(got, "git receive-pack '%s/foo.git'" % tmp) + def test_push_inits_if_needed(): # a push to a non-existent repository (but where config authorizes # you to do that) will create the repository on the fly -- cgit v1.2.3