Thread (26 messages) flat view 26 messages, 6 authors, 2016-06-15

Re: [PATCH] daemon: return "access denied" if a service is not allowed

From: Jeff King <hidden>
Date: 2016-06-15 22:52:15
Subsystem: documentation, the rest · Maintainers: Jonathan Corbet, Linus Torvalds

Possibly related (same subject, not in this thread)

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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help