Re: [PATCH] daemon: return "access denied" if a service is not allowed
- From
Jeff King <peff@peff.net>
- Date
- Oct 14, 2011, 19:23 UTC
- Message-ID
- <20111014192326.GA7713@sigill.intra.peff.net>
- In-Reply-To
- <20111014131041.GC7808@sigill.intra.peff.net>
On Fri, Oct 14, 2011 at 09:10:41AM -0400, Jeff King wrote:
> Let's start with my original patch (which I'll clean and repost). And if > somebody really wants to push towards customized messages, that is easy > enough to do on top later.
Here it is, with a few minor tweaks:
1. git-daemon respects --no-informative-errors, too
2. the new flag is documented
3. I switched the format from:
fatal: remote: /path/in/your/git/url: access denied
to:
fatal: remote: access denied: /path/in/your/git/url
I find the latter easier to read, and it would be easier for a
client to recognize.4. there's now a commit message
-- >8 -- Subject: [PATCH] daemon: give friendlier error messages to clients
When the git-daemon is asked about an inaccessible repository, it simply hangs up the connection without saying anything further. This makes it hard to distinguish between a repository we cannot access (e.g., due to typo), and a service or network outage.
Instead, let's print an "ERR" line, which git clients understand since v1.6.1 (2008-12-24).
Because there is a risk of leaking information about non-exported repositories, by default all errors simply say "access denied". Open sites can pass a flag to turn on more specific messages.
Signed-off-by: Jeff King <peff@peff.net> --- Documentation/git-daemon.txt | 8 ++++++++ daemon.c | 25 +++++++++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt index 69a1e4a..ac57c6d 100644 --- a/Documentation/git-daemon.txt +++ b/Documentation/git-daemon.txt @@ -161,6 +161,14 @@ the facility of inet daemon to achieve the same before spawning repository configuration. By default, all the services are overridable. +--informative-errors:: + Return more verbose errors to the client, differentiating + conditions like "no such repository" from "repository not + exported". This is more convenient for clients, but may leak + information about the existence of unexported repositories. + Without this option, all errors report "access denied" to the + client. + <directory>:: A directory to add to the whitelist of allowed directories. Unless --strict-paths is specified this will also include subdirectories diff --git a/daemon.c b/daemon.c index 4c8346d..e5869ec 100644 --- a/daemon.c +++ b/daemon.c @@ -20,6 +20,7 @@ static int log_syslog; static int verbose; static int reuseaddr; +static int informative_errors; static const char daemon_usage[] = "git daemon [--verbose] [--syslog] [--export-all]\n" @@ -247,6 +248,14 @@ static int git_daemon_config(const char *var, const char *value, void *cb) return 0; } +static int daemon_error(const char *dir, const char *msg) +{ + if (!informative_errors) + msg = "access denied"; + packet_write(1, "ERR %s: %s", msg, dir); + return -1; +} + static int run_service(char *dir, struct daemon_service *service) { const char *path; @@ -257,11 +266,11 @@ static int run_service(char *dir, struct daemon_service *service) if (!enabled && !service->overridable) { logerror("'%s': service not enabled.", service->name); errno = EACCES; - return -1; + return daemon_error(dir, "service not enabled"); } if (!(path = path_ok(dir))) - return -1; + return daemon_error(dir, "no such repository"); /* * Security on the cheap. @@ -277,7 +286,7 @@ static int run_service(char *dir, struct daemon_service *service) if (!export_all_trees && access("git-daemon-export-ok", F_OK)) { logerror("'%s': repository not exported.", path); errno = EACCES; - return -1; + return daemon_error(dir, "repository not exported"); } if (service->overridable) { @@ -291,7 +300,7 @@ static int run_service(char *dir, struct daemon_service *service) logerror("'%s': service not enabled for '%s'", service->name, path); errno = EACCES; - return -1; + return daemon_error(dir, "service not enabled"); } /* @@ -1167,6 +1176,14 @@ int main(int argc, char **argv) make_service_overridable(arg + 18, 0); continue; } + if (!prefixcmp(arg, "--informative-errors")) { + informative_errors = 1; + continue; + } + else if (!prefixcmp(arg, "--no-informative-errors")) { + informative_errors = 0; + continue; + } if (!strcmp(arg, "--")) { ok_paths = &argv[i+1]; break;
-- 1.7.6.4.37.g43b58b