* Does smart-http need git-daemon-export-ok? @ 2009-12-26 16:21 Tarmigan 2009-12-26 17:33 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Tarmigan @ 2009-12-26 16:21 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: Git Mailing List, Tay Ray Chuan, Clemens Buchacher, J.H. Hi all, Should the git-http-backend check something like git-daemon-export-ok before serving a repository? It would have been better to discuss this a month or two ago, but it's probably not too late since smart-http is still so new in released versions. I think it's very similar to git-daemon which requires that to be set, and I think the same arguments could be made for the same kind of check. There are already parallels for the upload-pack and receive-pack services between the two. Just as git-daemon may be invoked with --export-all, for git-http-backend we could have an environmental variable to export all repositories. Thoughts? Thanks, Tarmigan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Does smart-http need git-daemon-export-ok? 2009-12-26 16:21 Does smart-http need git-daemon-export-ok? Tarmigan @ 2009-12-26 17:33 ` Junio C Hamano 2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt 2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce 0 siblings, 2 replies; 14+ messages in thread From: Junio C Hamano @ 2009-12-26 17:33 UTC (permalink / raw) To: Tarmigan Cc: Shawn O. Pearce, Git Mailing List, Tay Ray Chuan, Clemens Buchacher, J.H. Tarmigan <tarmigan+git@gmail.com> writes: > Should the git-http-backend check something like git-daemon-export-ok > before serving a repository? I'd agree that it would make sense to have a way to mark individual repository for (or not for) export. In "native" case, the chain of events are: client talks to the daemon, the daemon checks and decides to (or not to) export, and it runs upload-pack. In "smart http" case, http-backend is one half of what corresponds to the daemon (the other half being your http server configuration), and it is more flexible and git specific half, so I'd say it would make sense to implement the check that honors the same git-daemon-export-ok flag file in it. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] Smart-http: Add tests and documentation for export-ok 2009-12-26 17:33 ` Junio C Hamano @ 2009-12-26 23:29 ` Tarmigan Casebolt 2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt 2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce 1 sibling, 1 reply; 14+ messages in thread From: Tarmigan Casebolt @ 2009-12-26 23:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: spearce, git, rctay89, drizzd, warthog9, Tarmigan Casebolt Add some tests for having smart-http check whether a repository is ok to export. Add tests for the GIT_HTTP_EXPORT_ALL environmental variable and checking the git-daemon-export-ok file, while leaving existing tests still functional. Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> --- Documentation/git-http-backend.txt | 10 +++++++++ t/lib-httpd/apache.conf | 5 ++++ t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt index 67aec06..c8fe08a 100644 --- a/Documentation/git-http-backend.txt +++ b/Documentation/git-http-backend.txt @@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol and the backwards-compatible dumb HTTP protocol, as well as clients pushing using the smart HTTP protocol. +It verifies that the directory has the magic file +"git-daemon-export-ok", and it will refuse to export any git directory +that hasn't explicitly been marked for export this way (unless the +GIT_HTTP_EXPORT_ALL environmental variable is set). + By default, only the `upload-pack` service is enabled, which serves 'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from 'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated, @@ -70,6 +75,7 @@ Apache 2.x:: + ---------------------------------------------------------------- SetEnv GIT_PROJECT_ROOT /var/www/git +SetEnv GIT_HTTP_EXPORT_ALL ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/ ---------------------------------------------------------------- + @@ -157,6 +163,10 @@ by the invoking web server, including: * QUERY_STRING * REQUEST_METHOD +The GIT_HTTP_EXPORT_ALL environmental variable may be passed to +'git-http-backend' to bypass the check for the "git-daemon-export-ok" +file in each repository before allowing export of that repository. + The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}', ensuring that any reflogs created by 'git-receive-pack' contain some diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 0fe3fd0..4961505 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -22,8 +22,13 @@ Alias /dumb/ www/ <Location /smart/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL +</Location> +<Location /smart_noexport/> + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} </Location> ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/ +ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/ <Directory ${GIT_EXEC_PATH}> Options None </Directory> diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh index ed034bc..f763880 100755 --- a/t/t5560-http-backend.sh +++ b/t/t5560-http-backend.sh @@ -23,7 +23,7 @@ config() { } GET() { - curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null && + curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null && tr '\015' Q <out | sed ' s/Q$// @@ -91,6 +91,20 @@ get_static_files() { GET $IDX_URL "$1" } +SMART=smart_noexport +test_expect_success 'no export by default' ' + log_div "no git-daemon-export-ok" + get_static_files "404 Not Found" +' +test_expect_success 'export if git-daemon-export-ok' ' + log_div "git-daemon-export-ok" + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + touch git-daemon-export-ok + ) && + get_static_files "200 OK" +' + +SMART=smart test_expect_success 'direct refs/heads/master not found' ' log_div "refs/heads/master" GET refs/heads/master "404 Not Found" @@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' ' GET info/refs?service=git-receive-pack "403 Forbidden" && POST git-receive-pack 0000 "403 Forbidden" ' - run_backend() { REQUEST_METHOD=GET \ GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \ @@ -179,6 +192,28 @@ test_expect_success 'http-backend blocks bad PATH_INFO' ' cat >exp <<EOF +### no git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 - +GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 - +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 - +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 - + +### git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 200 +GET /smart_noexport/repo.git/info/refs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200 + ### refs/heads/master ### GET /smart/repo.git/refs/heads/master HTTP/1.1 404 - -- 1.6.6.2.g5daf2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt @ 2009-12-26 23:29 ` Tarmigan Casebolt 2009-12-27 21:10 ` Shawn O. Pearce 0 siblings, 1 reply; 14+ messages in thread From: Tarmigan Casebolt @ 2009-12-26 23:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: spearce, git, rctay89, drizzd, warthog9, Tarmigan Casebolt Similar to how git-daemon checks whether a repository is OK to be exported, smart-http should also check. This check can be satisfied in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL may be set to export all repositories, or the individual repository may have the file git-daemon-export-ok. Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> --- http-backend.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/http-backend.c b/http-backend.c index f729488..345c12b 100644 --- a/http-backend.c +++ b/http-backend.c @@ -648,6 +648,9 @@ int main(int argc, char **argv) setup_path(); if (!enter_repo(dir, 0)) not_found("Not a git repository: '%s'", dir); + if (!getenv("GIT_HTTP_EXPORT_ALL") && + access("git-daemon-export-ok", F_OK) ) + not_found("Repository not exported: '%s'", dir); git_config(http_config, NULL); cmd->imp(cmd_arg); -- 1.6.6.2.g5daf2 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt @ 2009-12-27 21:10 ` Shawn O. Pearce 2009-12-28 4:07 ` Tarmigan 0 siblings, 1 reply; 14+ messages in thread From: Shawn O. Pearce @ 2009-12-27 21:10 UTC (permalink / raw) To: Tarmigan Casebolt; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9 Tarmigan Casebolt <tarmigan+git@gmail.com> wrote: > Similar to how git-daemon checks whether a repository is OK to be > exported, smart-http should also check. This check can be satisfied > in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL > may be set to export all repositories, or the individual repository > may have the file git-daemon-export-ok. > > Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> Acked-by: Shawn O. Pearce <spearce@spearce.org> I really think this and 1/2 should be squashed together, in which case you can apply my ACK to the entire thing. -- Shawn. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-27 21:10 ` Shawn O. Pearce @ 2009-12-28 4:07 ` Tarmigan 2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt 2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce 0 siblings, 2 replies; 14+ messages in thread From: Tarmigan @ 2009-12-28 4:07 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9 On Sun, Dec 27, 2009 at 4:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote: > Tarmigan Casebolt <tarmigan+git@gmail.com> wrote: >> Similar to how git-daemon checks whether a repository is OK to be >> exported, smart-http should also check. This check can be satisfied >> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL >> may be set to export all repositories, or the individual repository >> may have the file git-daemon-export-ok. >> >> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> > > Acked-by: Shawn O. Pearce <spearce@spearce.org> > > I really think this and 1/2 should be squashed together, in which > case you can apply my ACK to the entire thing. Great, thanks for the ACK. Squashing sounds good to me, I just split it so someone could verify that the tests fail first if they want. I've been thinking that the not_found() to a forbidden() instead. Thoughts? I'll send out a unified patch with that change in a reply. Thanks, Tarmigan ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Smart-http: check if repository is OK to export before serving it 2009-12-28 4:07 ` Tarmigan @ 2009-12-28 4:22 ` Tarmigan Casebolt 2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce 1 sibling, 0 replies; 14+ messages in thread From: Tarmigan Casebolt @ 2009-12-28 4:22 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Tarmigan Casebolt Similar to how git-daemon checks whether a repository is OK to be exported, smart-http should also check. This check can be satisfied in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL may be set to export all repositories, or the individual repository may have the file git-daemon-export-ok. Acked-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> --- Documentation/git-http-backend.txt | 10 +++++++++ http-backend.c | 3 ++ t/lib-httpd/apache.conf | 5 ++++ t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++- 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt index 67aec06..c8fe08a 100644 --- a/Documentation/git-http-backend.txt +++ b/Documentation/git-http-backend.txt @@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol and the backwards-compatible dumb HTTP protocol, as well as clients pushing using the smart HTTP protocol. +It verifies that the directory has the magic file +"git-daemon-export-ok", and it will refuse to export any git directory +that hasn't explicitly been marked for export this way (unless the +GIT_HTTP_EXPORT_ALL environmental variable is set). + By default, only the `upload-pack` service is enabled, which serves 'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from 'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated, @@ -70,6 +75,7 @@ Apache 2.x:: + ---------------------------------------------------------------- SetEnv GIT_PROJECT_ROOT /var/www/git +SetEnv GIT_HTTP_EXPORT_ALL ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/ ---------------------------------------------------------------- + @@ -157,6 +163,10 @@ by the invoking web server, including: * QUERY_STRING * REQUEST_METHOD +The GIT_HTTP_EXPORT_ALL environmental variable may be passed to +'git-http-backend' to bypass the check for the "git-daemon-export-ok" +file in each repository before allowing export of that repository. + The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}', ensuring that any reflogs created by 'git-receive-pack' contain some diff --git a/http-backend.c b/http-backend.c index f729488..9de85cb 100644 --- a/http-backend.c +++ b/http-backend.c @@ -648,6 +648,9 @@ int main(int argc, char **argv) setup_path(); if (!enter_repo(dir, 0)) not_found("Not a git repository: '%s'", dir); + if (!getenv("GIT_HTTP_EXPORT_ALL") && + access("git-daemon-export-ok", F_OK) ) + forbidden("Repository not exported: '%s'", dir); git_config(http_config, NULL); cmd->imp(cmd_arg); diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 0fe3fd0..4961505 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -22,8 +22,13 @@ Alias /dumb/ www/ <Location /smart/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL +</Location> +<Location /smart_noexport/> + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} </Location> ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/ +ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/ <Directory ${GIT_EXEC_PATH}> Options None </Directory> diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh index ed034bc..126f6d5 100755 --- a/t/t5560-http-backend.sh +++ b/t/t5560-http-backend.sh @@ -23,7 +23,7 @@ config() { } GET() { - curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null && + curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null && tr '\015' Q <out | sed ' s/Q$// @@ -91,6 +91,20 @@ get_static_files() { GET $IDX_URL "$1" } +SMART=smart_noexport +test_expect_success 'no export by default' ' + log_div "no git-daemon-export-ok" + get_static_files "403 Forbidden" +' +test_expect_success 'export if git-daemon-export-ok' ' + log_div "git-daemon-export-ok" + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + touch git-daemon-export-ok + ) && + get_static_files "200 OK" +' + +SMART=smart test_expect_success 'direct refs/heads/master not found' ' log_div "refs/heads/master" GET refs/heads/master "404 Not Found" @@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' ' GET info/refs?service=git-receive-pack "403 Forbidden" && POST git-receive-pack 0000 "403 Forbidden" ' - run_backend() { REQUEST_METHOD=GET \ GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \ @@ -179,6 +192,28 @@ test_expect_success 'http-backend blocks bad PATH_INFO' ' cat >exp <<EOF +### no git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 403 - +GET /smart_noexport/repo.git/info/refs HTTP/1.1 403 - +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 403 - +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 403 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 403 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 403 - +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 403 - +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 403 - + +### git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 200 +GET /smart_noexport/repo.git/info/refs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200 + ### refs/heads/master ### GET /smart/repo.git/refs/heads/master HTTP/1.1 404 - -- 1.6.6.1.g8eede.dirty ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-28 4:07 ` Tarmigan 2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt @ 2009-12-28 15:59 ` Shawn O. Pearce 2009-12-28 16:57 ` Tarmigan 1 sibling, 1 reply; 14+ messages in thread From: Shawn O. Pearce @ 2009-12-28 15:59 UTC (permalink / raw) To: Tarmigan; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9 Tarmigan <tarmigan+git@gmail.com> wrote: > On Sun, Dec 27, 2009 at 4:10 PM, Shawn O. Pearce <spearce@spearce.org> wrote: > > Tarmigan Casebolt <tarmigan+git@gmail.com> wrote: > >> Similar to how git-daemon checks whether a repository is OK to be > >> exported, smart-http should also check. ?This check can be satisfied > >> in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL > >> may be set to export all repositories, or the individual repository > >> may have the file git-daemon-export-ok. ... > I've been thinking that the not_found() to a forbidden() instead. Oh. Interesting question. Because you can't resolve the access error by authenticating to the server, we may actually want to just return not_found() here with a message in the log of "Repository not exported: '%s'". That would mirror the behavior of git-daemon. -- Shawn. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce @ 2009-12-28 16:57 ` Tarmigan 2009-12-28 17:08 ` Shawn O. Pearce 0 siblings, 1 reply; 14+ messages in thread From: Tarmigan @ 2009-12-28 16:57 UTC (permalink / raw) To: Shawn O. Pearce; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9 On Mon, Dec 28, 2009 at 10:59 AM, Shawn O. Pearce <spearce@spearce.org> wrote: > Tarmigan <tarmigan+git@gmail.com> wrote: >> I've been thinking that the not_found() to a forbidden() instead. > > Oh. Interesting question. > > Because you can't resolve the access error by authenticating to > the server, we may actually want to just return not_found() here > with a message in the log of "Repository not exported: '%s'". I'm no http expert, but isn't that what 401 would be? From http://tools.ietf.org/html/rfc2616#section-10.4.4 403 Forbidden The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead. which to me points to 403 instead of 404. I don't have a strong preference, and will resend with those changes if you'd prefer 404. Thanks, Tarmigan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] Smart-http: check if repository is OK to export before serving it 2009-12-28 16:57 ` Tarmigan @ 2009-12-28 17:08 ` Shawn O. Pearce 2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt 0 siblings, 1 reply; 14+ messages in thread From: Shawn O. Pearce @ 2009-12-28 17:08 UTC (permalink / raw) To: Tarmigan; +Cc: Junio C Hamano, git, rctay89, drizzd, warthog9 Tarmigan <tarmigan+git@gmail.com> wrote: > On Mon, Dec 28, 2009 at 10:59 AM, Shawn O. Pearce <spearce@spearce.org> wrote: > > Tarmigan <tarmigan+git@gmail.com> wrote: > >> I've been thinking that the not_found() to a forbidden() instead. > > > > Because you can't resolve the access error by authenticating to > > the server, we may actually want to just return not_found() here > > with a message in the log of "Repository not exported: '%s'". > > I'm no http expert, but isn't that what 401 would be? From > http://tools.ietf.org/html/rfc2616#section-10.4.4 > 403 Forbidden > The server understood the request, but is refusing to fulfill it. > Authorization will not help and the request SHOULD NOT be repeated. > If the request method was not HEAD and the server wishes to make > public why the request has not been fulfilled, it SHOULD describe the > reason for the refusal in the entity. If the server does not wish to > make this information available to the client, the status code 404 > (Not Found) can be used instead. > which to me points to 403 instead of 404. Good point, that is 403. But the last sentance leads me to believe 404 might be a better use here. Under git-daemon we don't tell the client the difference between "Not Found" and "Not Exported", so I think we should be doing the same thing here under HTTP. -- Shawn. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] Smart-http: check if repository is OK to export before serving it 2009-12-28 17:08 ` Shawn O. Pearce @ 2009-12-28 21:49 ` Tarmigan Casebolt 2009-12-29 9:19 ` Junio C Hamano 0 siblings, 1 reply; 14+ messages in thread From: Tarmigan Casebolt @ 2009-12-28 21:49 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, spearce, Tarmigan Casebolt Similar to how git-daemon checks whether a repository is OK to be exported, smart-http should also check. This check can be satisfied in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL may be set to export all repositories, or the individual repository may have the file git-daemon-export-ok. Acked-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> --- OK, I see what you're saying Shawn. I've changed it back to "404 Not Found" again. I've also reordered the new tests since the last time I sent it out. The new tests use the same test as in "static file is ok" so put the new tests after that test in case that test breaks. Documentation/git-http-backend.txt | 10 +++++++++ http-backend.c | 3 ++ t/lib-httpd/apache.conf | 5 ++++ t/t5560-http-backend.sh | 39 ++++++++++++++++++++++++++++++++++- 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/Documentation/git-http-backend.txt b/Documentation/git-http-backend.txt index 67aec06..c8fe08a 100644 --- a/Documentation/git-http-backend.txt +++ b/Documentation/git-http-backend.txt @@ -18,6 +18,11 @@ The program supports clients fetching using both the smart HTTP protcol and the backwards-compatible dumb HTTP protocol, as well as clients pushing using the smart HTTP protocol. +It verifies that the directory has the magic file +"git-daemon-export-ok", and it will refuse to export any git directory +that hasn't explicitly been marked for export this way (unless the +GIT_HTTP_EXPORT_ALL environmental variable is set). + By default, only the `upload-pack` service is enabled, which serves 'git-fetch-pack' and 'git-ls-remote' clients, which are invoked from 'git-fetch', 'git-pull', and 'git-clone'. If the client is authenticated, @@ -70,6 +75,7 @@ Apache 2.x:: + ---------------------------------------------------------------- SetEnv GIT_PROJECT_ROOT /var/www/git +SetEnv GIT_HTTP_EXPORT_ALL ScriptAlias /git/ /usr/libexec/git-core/git-http-backend/ ---------------------------------------------------------------- + @@ -157,6 +163,10 @@ by the invoking web server, including: * QUERY_STRING * REQUEST_METHOD +The GIT_HTTP_EXPORT_ALL environmental variable may be passed to +'git-http-backend' to bypass the check for the "git-daemon-export-ok" +file in each repository before allowing export of that repository. + The backend process sets GIT_COMMITTER_NAME to '$REMOTE_USER' and GIT_COMMITTER_EMAIL to '$\{REMOTE_USER}@http.$\{REMOTE_ADDR\}', ensuring that any reflogs created by 'git-receive-pack' contain some diff --git a/http-backend.c b/http-backend.c index f729488..345c12b 100644 --- a/http-backend.c +++ b/http-backend.c @@ -648,6 +648,9 @@ int main(int argc, char **argv) setup_path(); if (!enter_repo(dir, 0)) not_found("Not a git repository: '%s'", dir); + if (!getenv("GIT_HTTP_EXPORT_ALL") && + access("git-daemon-export-ok", F_OK) ) + not_found("Repository not exported: '%s'", dir); git_config(http_config, NULL); cmd->imp(cmd_arg); diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 0fe3fd0..4961505 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -22,8 +22,13 @@ Alias /dumb/ www/ <Location /smart/> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL +</Location> +<Location /smart_noexport/> + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} </Location> ScriptAlias /smart/ ${GIT_EXEC_PATH}/git-http-backend/ +ScriptAlias /smart_noexport/ ${GIT_EXEC_PATH}/git-http-backend/ <Directory ${GIT_EXEC_PATH}> Options None </Directory> diff --git a/t/t5560-http-backend.sh b/t/t5560-http-backend.sh index ed034bc..04a9896 100755 --- a/t/t5560-http-backend.sh +++ b/t/t5560-http-backend.sh @@ -23,7 +23,7 @@ config() { } GET() { - curl --include "$HTTPD_URL/smart/repo.git/$1" >out 2>/dev/null && + curl --include "$HTTPD_URL/$SMART/repo.git/$1" >out 2>/dev/null && tr '\015' Q <out | sed ' s/Q$// @@ -91,6 +91,7 @@ get_static_files() { GET $IDX_URL "$1" } +SMART=smart test_expect_success 'direct refs/heads/master not found' ' log_div "refs/heads/master" GET refs/heads/master "404 Not Found" @@ -99,6 +100,19 @@ test_expect_success 'static file is ok' ' log_div "getanyfile default" get_static_files "200 OK" ' +SMART=smart_noexport +test_expect_success 'no export by default' ' + log_div "no git-daemon-export-ok" + get_static_files "404 Not Found" +' +test_expect_success 'export if git-daemon-export-ok' ' + log_div "git-daemon-export-ok" + (cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + touch git-daemon-export-ok + ) && + get_static_files "200 OK" +' +SMART=smart test_expect_success 'static file if http.getanyfile true is ok' ' log_div "getanyfile true" config http.getanyfile true && @@ -145,7 +159,6 @@ test_expect_success 'http.receivepack false' ' GET info/refs?service=git-receive-pack "403 Forbidden" && POST git-receive-pack 0000 "403 Forbidden" ' - run_backend() { REQUEST_METHOD=GET \ GIT_PROJECT_ROOT="$HTTPD_DOCUMENT_ROOT_PATH" \ @@ -194,6 +207,28 @@ GET /smart/repo.git/$LOOSE_URL HTTP/1.1 200 GET /smart/repo.git/$PACK_URL HTTP/1.1 200 GET /smart/repo.git/$IDX_URL HTTP/1.1 200 +### no git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 404 - +GET /smart_noexport/repo.git/info/refs HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 404 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 404 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 404 - +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 404 - +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 404 - + +### git-daemon-export-ok +### +GET /smart_noexport/repo.git/HEAD HTTP/1.1 200 +GET /smart_noexport/repo.git/info/refs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/packs HTTP/1.1 200 +GET /smart_noexport/repo.git/objects/info/alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/objects/info/http-alternates HTTP/1.1 200 - +GET /smart_noexport/repo.git/$LOOSE_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$PACK_URL HTTP/1.1 200 +GET /smart_noexport/repo.git/$IDX_URL HTTP/1.1 200 + ### getanyfile true ### GET /smart/repo.git/HEAD HTTP/1.1 200 -- 1.6.6.1.g8d7b9 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] Smart-http: check if repository is OK to export before serving it 2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt @ 2009-12-29 9:19 ` Junio C Hamano 2009-12-29 15:00 ` Shawn O. Pearce 0 siblings, 1 reply; 14+ messages in thread From: Junio C Hamano @ 2009-12-29 9:19 UTC (permalink / raw) To: Tarmigan Casebolt; +Cc: git, spearce Tarmigan Casebolt <tarmigan+git@gmail.com> writes: > Similar to how git-daemon checks whether a repository is OK to be > exported, smart-http should also check. This check can be satisfied > in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL > may be set to export all repositories, or the individual repository > may have the file git-daemon-export-ok. > > Acked-by: Shawn O. Pearce <spearce@spearce.org> > Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> > --- > OK, I see what you're saying Shawn. I've changed it back to "404 > Not Found" again. > > I've also reordered the new tests since the last time I sent it out. > The new tests use the same test as in > "static file is ok" > so put the new tests after that test in case that test breaks. Looks sane to me, although I am afraid that I am not as familiar with the codepath involved as I should be. Shawn, is your Ack still good? ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] Smart-http: check if repository is OK to export before serving it 2009-12-29 9:19 ` Junio C Hamano @ 2009-12-29 15:00 ` Shawn O. Pearce 0 siblings, 0 replies; 14+ messages in thread From: Shawn O. Pearce @ 2009-12-29 15:00 UTC (permalink / raw) To: Junio C Hamano; +Cc: Tarmigan Casebolt, git Junio C Hamano <gitster@pobox.com> wrote: > Tarmigan Casebolt <tarmigan+git@gmail.com> writes: > > > Similar to how git-daemon checks whether a repository is OK to be > > exported, smart-http should also check. This check can be satisfied > > in two different ways: the environmental variable GIT_HTTP_EXPORT_ALL > > may be set to export all repositories, or the individual repository > > may have the file git-daemon-export-ok. > > > > Acked-by: Shawn O. Pearce <spearce@spearce.org> ... > Looks sane to me, although I am afraid that I am not as familiar with the > codepath involved as I should be. Shawn, is your Ack still good? Yes, my ACK is still good. :-) -- Shawn. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Does smart-http need git-daemon-export-ok? 2009-12-26 17:33 ` Junio C Hamano 2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt @ 2009-12-27 21:06 ` Shawn O. Pearce 1 sibling, 0 replies; 14+ messages in thread From: Shawn O. Pearce @ 2009-12-27 21:06 UTC (permalink / raw) To: Junio C Hamano Cc: Tarmigan, Git Mailing List, Tay Ray Chuan, Clemens Buchacher, J.H. Junio C Hamano <gitster@pobox.com> wrote: > Tarmigan <tarmigan+git@gmail.com> writes: > > Should the git-http-backend check something like git-daemon-export-ok > > before serving a repository? > > I'd agree that it would make sense to have a way to mark individual > repository for (or not for) export. Just for some background... early drafts of git-http-backend actually did check for, and require, this file before it exported a repository. I took the check out because I was relying on the HTTP server's document root translation to provide the mapping into the local filesystem. That meant the HTTP repository was already exported via dumb-http, and the git-daemon-export-ok flag wasn't being checked. Later in the series development we got the patch to allow a different filesystem root via an environment variable, which means its possible to hide repositories and make them available only through git-http-backend. In that configuration, checking the git-daemon-export-ok flag makes sense again. > In "native" case, the chain of events are: client talks to the daemon, the > daemon checks and decides to (or not to) export, and it runs upload-pack. > > In "smart http" case, http-backend is one half of what corresponds to the > daemon (the other half being your http server configuration), and it is > more flexible and git specific half, so I'd say it would make sense to > implement the check that honors the same git-daemon-export-ok flag file in > it. Yea, I'd agree. -- Shawn. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-12-29 15:00 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-26 16:21 Does smart-http need git-daemon-export-ok? Tarmigan 2009-12-26 17:33 ` Junio C Hamano 2009-12-26 23:29 ` [PATCH 1/2] Smart-http: Add tests and documentation for export-ok Tarmigan Casebolt 2009-12-26 23:29 ` [PATCH 2/2] Smart-http: check if repository is OK to export before serving it Tarmigan Casebolt 2009-12-27 21:10 ` Shawn O. Pearce 2009-12-28 4:07 ` Tarmigan 2009-12-28 4:22 ` [PATCH] " Tarmigan Casebolt 2009-12-28 15:59 ` [PATCH 2/2] " Shawn O. Pearce 2009-12-28 16:57 ` Tarmigan 2009-12-28 17:08 ` Shawn O. Pearce 2009-12-28 21:49 ` [PATCH] " Tarmigan Casebolt 2009-12-29 9:19 ` Junio C Hamano 2009-12-29 15:00 ` Shawn O. Pearce 2009-12-27 21:06 ` Does smart-http need git-daemon-export-ok? Shawn O. Pearce
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).