Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:52:15
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] daemon: return "access denied" if a service is not allowed · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH] daemon: return "access denied" if a service is not allowed · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] daemon: return "access denied" if a service is not allowed · Jeff King <hidden>
- 2016-06-15 · Re: [PATCH] daemon: return "access denied" if a service is not allowed · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] daemon: return "access denied" if a service is not allowed · Jeff King <hidden>
Jeff King wrote:
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.
*nod*
Instead, let's print an "ERR" line, which git clients understand since v1.6.1 (2008-12-24).
Just to be clear, "git archive --remote" does not understand ERR lines in the 'master' branch (though 908aaceb makes it understand them in 'next'). But I consider even distinguishing a. fatal: git archive: protocol error b. fatal: git archive: expected ACK/NAK, got EOF [(a) is how an ERR response is reported, and (b) a remote hangup] to be progress, so it's not so important. :)
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.
I'm not sure what an "open site" is. :) But having this flag for sites to declare whether they consider whether a repository exists to be privileged information seems reasonable to me. Note that this really would be privileged information in some not-too-weird cases. For example, if many users have a repository at ~/.git, ~/.config/.git, or ~/src/linux/.git, then someone might try to access /home/alice/.git /home/alice/.config/.git /home/alice/src/linux/.git /home/bob/.git ... in turn to find a valid username, as reconnaisance for a later attack not involving git. Luckily, this can be avoided with git daemon --user-path=public_git --base-path=/pub/git which only allows access to subdirectories of /pub/git and public_git in home directories. With git daemon --base-path=/pub/git there is still no problem, since access to home directories is not allowed at all in that case. I suppose the documentation should mention that --informative-errors is best not used unless --base-path is also in use. (Almost everyone is already using --base-path, so this isn't a very serious problem.)
quoted hunk ↗ jump to hunk
--- a/daemon.c +++ b/daemon.c@@ -20,6 +20,7 @@
[...]
quoted hunk ↗ jump to hunk
@@ -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, "--")) {
Micronit: uncuddled "else". The style of the surrounding code is to just not include the "else" at all and rely on "continue" to short-circuit things. Anyway, except for the documentation nits mentioned above (and Junio's nit, too), Reviewed-by: Jonathan Nieder <redacted> Thanks a lot for this.