Re: [PATCH v2 2/2] daemon: report permission denied error to clients
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:18
Clemens Buchacher [off-list ref] writes:
quoted hunk
diff --git a/daemon.c b/daemon.c index 72fb53a..2f7f84e 100644 --- a/daemon.c +++ b/daemon.c@@ -120,12 +120,14 @@ static char *path_ok(char *directory) if (daemon_avoid_alias(dir)) { logerror("'%s': aliased", dir); + errno = 0; return NULL; } if (*dir == '~') { if (!user_path) { logerror("'%s': User-path not allowed", dir); + errno = EACCES; return NULL; }
Isn't the first one inconsistent from all the others? A request cames "../some/path" and it is not allowed by a daemon policy and it gets errno==0 which is turned into "no such repo" later, while another request to "~drizzed/another/path" is also rejected by a daemon policy and gets errno==EACCESS which is turned into "permission denied". Indeed everything else says EACCESS in this patch, except for the check done by enter_repo() which can additionally say ENAMETOOLONG (which would not be very useful in practice) or whatever error coming from failure to go there with chdir(), which is not likely to be EACCESS because it has already been checked with a separate access() that is done before the actual chdir() call.
+ if (!(path = path_ok(dir))) {
+ if (errno == EACCES)
+ return daemon_error(dir, "permission denied");
+ else
+ return daemon_error(dir, "no such repository");
+ }
If errno is set to EACCESS in cases (1) we are not even going to tell you
if a repository exists there or not--you are not authorized to know and
(2) there is a repository but you do not have authorization to access it,
then this "leaking a bit more information" part is acceptable for site
with "--informative-errors", I would think. A repository that is invalid
from the daemon's point of view (e.g. validate_headref("HEAD") fails
because it points at an object that does not exist) but that the owner
intended to make it valid by correcting such mistakes would be reported as
"no such repository" with such a logic, so I am not sure if the distinction
between these two cases really matters in practice, though.