Re: [PATCH] daemon: return "access denied" if a service is not allowed
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(-)
Show changes to 2 files +29 −4
Documentation/git-daemon.txt, daemon.c
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
Nguyễn Thái Ngọc Duy
