All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Clemens Buchacher <drizzd@aon.at>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] daemon: report permission denied error to clients
Date: Mon, 17 Oct 2011 14:03:03 -0700	[thread overview]
Message-ID: <7vhb37l4ag.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20111017020912.GB18536@sigill.intra.peff.net

Jeff King <peff@peff.net> writes:

> On Mon, Oct 17, 2011 at 12:11:16AM +0200, Clemens Buchacher wrote:
>
> I like the intent. This actually does leak a little more information
> than the existing --informative-errors, as before you couldn't tell the
> difference between "not found" and "not exported".

I personally think this is going a bit too far, even for "informative"
option, by allowing to fish for possible list of usernames. It would make
it a tougher sell to later default to "informative", I am afraid.

Suppose you are setting up your own repository (either on your own box or
on a box with a separate administrator), and youare wondering why your
attempted access failed. You know /pub/repo/sito.git exists (you created
it after all) and you get "no such repository: /repo/sito.git" when you
ran:

    $ git clone git://host/repo/sito.git/

If you have another repository in /pub/repo/ that does already work, and
if you know /pub/repo/sito.git/ is fine locally (e.g. you can see local
command like "git log" works fine there), then even if you see "not found"
you would know to compare what the difference between these two are.

If there is no other repositories in /pub/repo/ or you are setting up many
repositories on this same box for the first time, wouldn't it be plausible
that you _are_ the administrator of the box and have access to the daemon
log to diagnose the problem more easily anyway?

I can see how this is "leaking a little more information", but I am not
convinced that leak is helping legit users more than helping unwanted
snoopers.

> The new calling conventions for this function seem a little weird.  I
> would expect either "return negative, and set errno" for usual library
> code, or possibly "return negative error value". But "return -1, or a
> positive error code" seems unusual to me.
>
> One of:
>
>   errno = EACCESS;
>   return -1;
>
> or
>
>   return -EACCESS;
>
> would be more idiomatic, I think.

Yes, the former would probably be easier to handle.

  parent reply	other threads:[~2011-10-17 21:03 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-01  1:26 [PATCH] transport: do not allow to push over git:// protocol Nguyễn Thái Ngọc Duy
2011-10-01  2:25 ` Ilari Liusvaara
2011-10-01  4:27   ` Nguyen Thai Ngoc Duy
2011-10-01  5:29   ` Jonathan Nieder
2011-10-03  9:12     ` Nguyen Thai Ngoc Duy
     [not found] ` <20111002223805.0bd6678b@zappedws>
2011-10-02 21:11   ` Nguyen Thai Ngoc Duy
2011-10-03  7:42 ` Jeff King
2011-10-03  8:44   ` Johannes Sixt
2011-10-03  9:39     ` Jeff King
2011-10-03  9:44       ` Nguyen Thai Ngoc Duy
2011-10-03  9:47         ` Jeff King
2011-10-03  9:52           ` Nguyen Thai Ngoc Duy
2011-10-03 11:13         ` Jonathan Nieder
2011-10-03 19:28           ` [PATCH] daemon: print "access denied" if a service does not work Nguyễn Thái Ngọc Duy
2011-10-03 19:54             ` Jonathan Nieder
2011-10-03 19:57             ` Junio C Hamano
2011-10-03 21:55               ` [PATCH] daemon: return "access denied" if a service is not allowed Nguyễn Thái Ngọc Duy
2011-10-03 22:20                 ` Junio C Hamano
2011-10-12 20:09                 ` Jeff King
2011-10-13  2:14                   ` Jonathan Nieder
2011-10-13  4:45                   ` Nguyen Thai Ngoc Duy
2011-10-13  5:59                     ` Jonathan Nieder
2011-10-13  6:56                       ` Nguyen Thai Ngoc Duy
2011-10-13  7:02                         ` Nguyen Thai Ngoc Duy
2011-10-13 18:28                     ` Jeff King
2011-10-14  5:01                       ` Junio C Hamano
2011-10-14 13:10                         ` Jeff King
2011-10-14 19:23                           ` Jeff King
2011-10-14 19:27                             ` Jeff King
2011-10-14 20:24                               ` Junio C Hamano
2011-10-14 20:34                                 ` Jeff King
2011-10-14 20:48                                   ` Junio C Hamano
2011-10-14 21:05                                     ` Jeff King
2011-10-14 21:06                                       ` Jonathan Nieder
2011-10-14 21:20                               ` Jonathan Nieder
2011-10-14 21:02                             ` Jonathan Nieder
2011-10-14 21:12                               ` Jeff King
2011-10-14 21:19                                 ` [PATCHv3] daemon: give friendlier error messages to clients Jeff King
2011-10-14 21:52                                   ` Junio C Hamano
2011-10-14 23:39                                   ` Sitaram Chamarty
2011-10-15  5:55                                     ` Junio C Hamano
2011-10-15  7:09                                       ` Sitaram Chamarty
2011-10-15  8:16                                         ` Jakub Narebski
2011-10-15  8:26                                           ` Jonathan Nieder
2011-10-15 20:13                                             ` Junio C Hamano
2011-10-15 22:17                                               ` Jonathan Nieder
2011-10-16  1:51                                                 ` Sitaram Chamarty
2011-10-15  0:51                                   ` Nguyen Thai Ngoc Duy
2011-10-16 22:11                                   ` [PATCH 1/2] daemon: add tests Clemens Buchacher
2011-10-16 22:11                                     ` [PATCH 2/2] daemon: report permission denied error to clients Clemens Buchacher
2011-10-17  2:09                                       ` Jeff King
2011-10-17 19:48                                         ` Clemens Buchacher
2011-10-17 19:51                                           ` Jeff King
2011-10-17 21:03                                         ` Junio C Hamano [this message]
2011-10-18 20:41                                           ` Clemens Buchacher
2011-10-19  6:33                                             ` Clemens Buchacher
2011-10-17 19:58                                       ` [PATCH v2 " Clemens Buchacher
2011-10-21 19:25                                         ` Junio C Hamano
2011-10-17  2:01                                     ` [PATCH 1/2] daemon: add tests Jeff King
2011-10-17 19:55                                       ` [PATCH] use test number as port number Clemens Buchacher
2011-10-17 20:57                                         ` Junio C Hamano
2011-10-18 20:09                                           ` Clemens Buchacher
2011-10-17 20:05                                       ` [PATCH 1/2] daemon: add tests Clemens Buchacher
2011-10-17 20:08                                         ` Jeff King
2012-01-02  9:25                                     ` Jonathan Nieder
2012-01-02 19:47                                       ` Clemens Buchacher
2012-01-03 19:18                                         ` Jeff King
2012-01-03 19:34                                       ` Junio C Hamano
2012-01-04 15:55                                         ` Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 1/6] t5550: repack everything into one file Clemens Buchacher
2012-01-04 18:05                                             ` Junio C Hamano
2012-01-04 15:55                                           ` [PATCH 2/6] daemon: add tests Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 3/6] avoid use of pkill Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 4/6] explain expected exit code Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 5/6] t5570: repack everything into one file Clemens Buchacher
2012-01-04 15:55                                           ` [PATCH 6/6] chmod: use lower-case x Clemens Buchacher
2012-01-04 18:00                                           ` [PATCH 1/2] daemon: add tests Junio C Hamano
2012-01-04 20:13                                             ` Junio C Hamano
2012-01-04 20:40                                             ` Clemens Buchacher
2012-01-04 22:15                                               ` Junio C Hamano
2012-01-04 22:26                                                 ` Jeff King
2012-01-05  0:07                                                   ` Clemens Buchacher
2012-01-05  0:24                                                     ` Junio C Hamano
2012-01-05  0:38                                                       ` Clemens Buchacher
2012-01-05  2:55                                                     ` Jeff King
2012-01-05 16:06                                                       ` Clemens Buchacher
2012-01-06 15:52                                                         ` Jeff King
2012-01-06 19:48                                                           ` Clemens Buchacher
2012-01-06 22:32                                                             ` Jeff King
2012-01-07 11:54                                                               ` [PATCH] credentials: unable to connect to cache daemon Clemens Buchacher
2012-01-07 14:55                                                                 ` Jeff King
2012-01-06 22:49                                                             ` [PATCH 1/2] daemon: add tests Junio C Hamano
2012-01-07 11:42                                                               ` Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 1/5] run-command: optionally kill children on exit Clemens Buchacher
2012-01-07 12:45                                                                   ` Erik Faye-Lund
2012-01-08 20:56                                                                     ` Clemens Buchacher
2012-01-07 14:41                                                                   ` Jeff King
2012-01-07 11:42                                                                 ` [PATCH 2/5] run-command: kill children on exit by default Clemens Buchacher
2012-01-07 14:50                                                                   ` Jeff King
2012-01-08  6:26                                                                     ` Junio C Hamano
2012-01-08 20:41                                                                       ` [PATCH 2/5 v2] dashed externals: kill children on exit Clemens Buchacher
2012-01-08 21:07                                                                         ` Jeff King
2012-01-07 11:42                                                                 ` [PATCH 3/5] git-daemon: add tests Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 4/5] git-daemon: produce output when ready Clemens Buchacher
2012-01-07 11:42                                                                 ` [PATCH 5/5] git-daemon tests: wait until daemon is ready Clemens Buchacher
2012-01-05  2:24                                                   ` [PATCH 1/2] daemon: add tests Jakub Narebski
2012-01-05  2:51                                                     ` Jeff King
2012-01-06 23:35                                                       ` Jakub Narebski
2012-01-07 11:46                                                         ` Clemens Buchacher
2012-01-06  6:17                                           ` Brian Gernhardt
2011-10-03  9:49     ` [PATCH] transport: do not allow to push over git:// protocol Jakub Narebski
2011-10-03 10:02       ` Jeff King
2011-10-03 11:01   ` Ilari Liusvaara
2011-10-03 11:26     ` [PATCH] Support ERR in remote archive like in fetch/push Jonathan Nieder
2011-10-03 11:45       ` René Scharfe
2011-10-03 18:13     ` [PATCH] transport: do not allow to push over git:// protocol Nguyen Thai Ngoc Duy
2011-10-03 20:27       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vhb37l4ag.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.