List for cgit developers and users
 help / color / mirror / Atom feed
From: mathstuf at gmail.com (Ben Boeckel)
Subject: [PATCHv4 2/2] Helper script to interface to gitolite
Date: Wed, 31 Oct 2012 23:23:56 -0400	[thread overview]
Message-ID: <20121101032356.GA29338@sir-slippy> (raw)
In-Reply-To: <20121101030301.GA28025@neptune.phub.net.cable.rogers.com>

On Wed, Oct 31, 2012 at 23:03:01 -0400, Jamie Couture wrote:
> What about users that have installed gitolite via their distro's
> package manager, as opposed to local install?

FreeBSD installs to /usr/local, so the path is fine for it.

> I do not think the script should assume to know where gitolite lives.
> 
> This might be agnostic:
> prog="$(which gitolite)"

It could also be configured with $PREFIX as part of the build. Any
hard-coded path is going to be wrong somewhere, so I think this might be
for distros to decide (does cgit even install contrib/ stuff by
default?). However...

> But that could lead to PATH abuse; potential security problems.

if you've injected something into $PATH as the user running the CGI
program, that's likely a system setup misconfiguration and I doubt
everything down that rabbit hole could be protected against ("Oops,
you're using an Apache setup vulnerable to BEAST; let's mitigate...").
If attackers are setting PATH somehow, there are other issues beyond
cgit's scope. Since cgit doesn't (and if it does, that probably needs to
not happen) touch PATH, it therefore shouldn't be responsible for
securing it.

> Maybe obtaining the value from another environment variable, say in
> your web server's virtual host:
> 
> SetEnv GITOLITE_PROG /path/to/gitolite
> 
> ---
> 
> prog="${GITOLITE_PROG}"

Assuming cgit doesn't touch PATH, this is just as valid an attack target
if PATH is an attack vector. Nothing is gained by making another
environment variable.

> Not sure how much of a crutch that is, but I wouldn't want the helper
> script to assume where gitolite lives; and if any distro were to
> include these scripts people might want it to work out of the box.

The solutions I see are:

  - hook it up to the build system (if contrib is already a part of it);
  - use `which`; or
  - ignore it and just have downstreams patch it for the system paths.

I'm in favor of the second one because it's the simplest solution. (I
don't think another patchset would be required if this is the only
thing; a commit on top of these should be fine; --amend should be fine
as well).

--Ben




  reply	other threads:[~2012-11-01  3:23 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-16  9:15 [PATCH 0/3] Implement authorization via external program valentin.haenel
2012-10-16  9:15 ` [PATCH 1/3] Add config option user-envvar valentin.haenel
2012-10-28  1:11   ` Jason
2012-10-29  9:49     ` valentin.haenel
2012-10-16  9:15 ` [PATCH 2/3] Add ability to authorize viewing a repository valentin.haenel
2012-10-16 11:21   ` Jason
2012-10-16 12:48     ` valentin.haenel
2012-10-16  9:15 ` [PATCH 2/3] Add ability to authorize viewing a repository using valentin.haenel
2012-10-16  9:17   ` valentin.haenel
2012-10-16  9:15 ` [PATCH 3/3] Helper script to interface to gitolite valentin.haenel
2012-10-22  8:29 ` [PATCHv2 1/3] Add config option user-envvar valentin.haenel
2012-10-28  1:00   ` mathstuf
2012-10-29  9:22     ` valentin.haenel
2012-10-29 14:53       ` mathstuf
2012-10-29 15:50         ` valentin.haenel
2012-10-22  8:29 ` [PATCHv2 2/3] Add ability to authorize viewing a repository valentin.haenel
2012-10-28  1:00   ` mathstuf
2012-10-28  1:16     ` Jason
2012-10-28  1:29       ` mathstuf
2012-10-28  1:33         ` Jason
2012-10-29  9:43       ` valentin.haenel
2012-10-29 14:52         ` mathstuf
2012-10-29 15:45           ` valentin.haenel
2012-10-28  1:17     ` Jason
2012-10-29 12:38       ` valentin.haenel
2012-10-30  9:54         ` valentin.haenel
2012-10-28  1:14   ` Jason
2012-10-29  9:36     ` valentin.haenel
2012-10-22  8:29 ` [PATCHv2 3/3] Helper script to interface to gitolite valentin.haenel
2012-10-28  1:00   ` mathstuf
2012-10-29  9:27     ` valentin.haenel
2012-10-30 10:11 ` [PATCHv3 0/3] Implement authorization via external program (v3) valentin.haenel
2012-10-30 15:04   ` mathstuf
2012-10-30 16:30   ` Jason
2012-10-30 10:11 ` [PATCHv3 1/3] Add config option user-envvar valentin.haenel
2012-10-30 16:29   ` Jason
2012-10-30 10:11 ` [PATCHv3 2/3] Add ability to authorize viewing a repository valentin.haenel
2012-10-30 16:30   ` Jason
2012-10-30 10:11 ` [PATCHv3 3/3] Helper script to interface to gitolite valentin.haenel
2012-10-30 15:05   ` mathstuf
2012-10-31 18:50 ` [PATCHv4 0/2] Authorize viewing a repository valentin.haenel
2012-10-31 18:52   ` [PATCHv4 1/2] Add ability to authorize " valentin.haenel
2012-10-31 18:52   ` [PATCHv4 2/2] Helper script to interface to gitolite valentin.haenel
2012-11-01  3:03     ` jamie.couture
2012-11-01  3:23       ` mathstuf [this message]
2012-11-01  4:20         ` Jason
2012-11-01  4:31           ` mathstuf
2012-11-01  8:58           ` valentin.haenel
2012-11-01 17:32             ` Jason
2012-11-01 10:40   ` [PATCHv4 0/2] Authorize viewing a repository valentin.haenel
2012-11-01 17:27     ` Jason
2012-11-01 17:32       ` valentin.haenel

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=20121101032356.GA29338@sir-slippy \
    --to=cgit@lists.zx2c4.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).