Re: [PATCH v2 2/2] daemon: report permission denied error to clients

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Clemens Buchacher <drizzd@xxxxxx> writes:

> 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.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]