[PATCH] git-receive-pack needs to set umask(2)

[PATCH] git-receive-pack needs to set umask(2)

[Michael Richardson]

[-- Attachment #1: Type: text/plain, Size: 3351 bytes --]


If there is another way to solve this, please let me know.
Wrapping git-receive-pack with a shell script to call umask seemed like too
global a change.
(also http://git.openswan.org/git#umask_hack)

When working with a common git repository, not all users are always clueful
enough to set their umask properly --- nor should the default for the user
always be so permissive.

This change adds $GIT_DIR/umask to contain a single line, an integer
which will be fed to umask(). This should also work for the git daemon,
which I personally do not use, so this may be inappropriate.

Signed-off-by: Michael Richardson <mcr@xelerance.com>

---

8698daf8fedc8618593ec44574df1efb9f31db84
 Documentation/git-receive-pack.txt |    3 +++
 cache.h                            |    1 +
 path.c                             |    2 ++
 setup.c                            |   19 +++++++++++++++++++
 4 files changed, 25 insertions(+), 0 deletions(-)

8698daf8fedc8618593ec44574df1efb9f31db84
diff --git a/Documentation/git-receive-pack.txt b/Documentation/git-receive-pack.txt
index 60debca..d3a8c11 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -74,6 +74,9 @@ packed and is served via a dumb transpor
 There are other real-world examples of using update and
 post-update hooks found in the Documentation/howto directory.
 
+The file $GIT_DIR/umask, if it exists will be opened, and the integer found
+in it will be used to initialize the umask(2) for subsequent file creation
+operations. 
 
 OPTIONS
 -------
diff --git a/cache.h b/cache.h
index 3a46fb9..65d5124 100644
--- a/cache.h
+++ b/cache.h
@@ -355,6 +355,7 @@ extern int git_config_bool(const char *,
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int check_repository_format_version(const char *var, const char *value);
+extern void setup_umask();
 
 #define MAX_GITNAME (1000)
 extern char git_default_email[MAX_GITNAME];
diff --git a/path.c b/path.c
index 334b2bd..571ff01 100644
--- a/path.c
+++ b/path.c
@@ -244,6 +244,8 @@ char *enter_repo(char *path, int strict)
 	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
 	    validate_symref("HEAD") == 0) {
 		putenv("GIT_DIR=.");
+
+		setup_umask();
 		check_repository_format();
 		return path;
 	}
diff --git a/setup.c b/setup.c
index fe7f884..2129125 100644
--- a/setup.c
+++ b/setup.c
@@ -228,6 +228,25 @@ int check_repository_format_version(cons
        return 0;
 }
 
+void setup_umask(void)
+{
+	FILE *f;
+
+	f = fopen(git_path("umask"), "r");
+	if(f != NULL) {
+		char maskstr[32];
+		if(fgets(maskstr, sizeof(maskstr), f) != NULL) {
+			char *foo;
+			unsigned int mask = strtoul(maskstr, &foo, 0);
+			
+			if(foo != maskstr) {
+				umask(mask);
+			}
+		}
+		fclose(f);
+	}
+}
+
 int check_repository_format(void)
 {
 	git_config(check_repository_format_version);
-- 
1.3.GIT





-- 
]       ON HUMILITY: to err is human. To moo, bovine.           |  firewalls  [
]   Michael Richardson,    Xelerance Corporation, Ottawa, ON    |net architect[
] mcr@xelerance.com      http://www.sandelman.ottawa.on.ca/mcr/ |device driver[
] panic("Just another Debian GNU/Linux using, kernel hacking, security guy"); [

    "The Microsoft _Get the Facts CD_ does not work on Linux." - orospakr


[-- Attachment #2: Type: application/pgp-signature, Size: 482 bytes --]

Re: [PATCH] git-receive-pack needs to set umask(2)

[Jakub Narebski]

Michael Richardson wrote:

> This change adds $GIT_DIR/umask to contain a single line, an integer
> which will be fed to umask(). This should also work for the git daemon,
> which I personally do not use, so this may be inappropriate.

Shouldn't it be done rather via $GIT_DIR/config file, and 
git-repo-config? I.e. instead of adding new file to repository layout,
$GIT_DIR/umask, add core.umask to git configuration?

-- 
Jakub Narebski
Warsaw, Poland

Re: [PATCH] git-receive-pack needs to set umask(2)

[Petr Baudis]

Dear diary, on Sun, May 28, 2006 at 11:31:41PM CEST, I got a letter
where Michael Richardson <mcr@sandelman.ottawa.on.ca> said that...
> If there is another way to solve this, please let me know.

Well, you didn't write what do you actually want to solve. Why do you
need to fiddle with the umask at all?

The object database is considered "append-only" unless you do git-prune
(and you should better not let anyone do that), thus it's enough if you
set all directories group-writable. Other than access the object
database, the users probably only want to update the refs - the solution
is to make refs/heads/ and refs/tags/ group-writable and setgid. This is
also what git-init-db --shared (or tools like cg-admin-setuprepo) should
already set up for you.

So, what did break?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
A person is just about as big as the things that make them angry.

Re: [PATCH] git-receive-pack needs to set umask(2)

[Johannes Schindelin]

Hi,

On Mon, 29 May 2006, Jakub Narebski wrote:

> Michael Richardson wrote:
> 
> > This change adds $GIT_DIR/umask to contain a single line, an integer
> > which will be fed to umask(). This should also work for the git daemon,
> > which I personally do not use, so this may be inappropriate.
> 
> Shouldn't it be done rather via $GIT_DIR/config file, and 
> git-repo-config? I.e. instead of adding new file to repository layout,
> $GIT_DIR/umask, add core.umask to git configuration?

See also

http://thread.gmane.org/gmane.comp.version-control.git/13856/focus=13876

The essence of the thread: If you want to do anything useful in a non-bare 
repository, you are likely using other tools than git, which do not 
interpret core.umask or $GIT_DIR/umask.

If you use a bare repository, just make it shared. No need for an umask.

Hth,
Dscho

Re: [PATCH] git-receive-pack needs to set umask(2)

[Salikh Zakirov]

Johannes Schindelin wrote:
> See also
> 
> http://thread.gmane.org/gmane.comp.version-control.git/13856/focus=13876

I've read the thread, but couldn't find a practical solution there.
 
> The essence of the thread: If you want to do anything useful in a non-bare 
> repository, you are likely using other tools than git, which do not 
> interpret core.umask or $GIT_DIR/umask.
> 
> If you use a bare repository, just make it shared. No need for an umask.

Could you please elaborate on what does it mean "make it shared"?

My setup: I have a bare GIT repository on a machine, where everybody can
SSH into (with full shell access). I've assigned the repo to a special group
where everybody belongs, and done a 'find repo.git -type d | xargs chmod 2775'

The problem: After someone pushed to the repository, the object directories 
(i.e repo.git/objects/??)
get created with 755 access rights, and effectively prevent everyone else from pushing
objects starting with the same prefix.

The obvious solution to use umask 002 is not applicable, because
1) It does not seem practical to enforce umask 002 in everyone's rc files, 
because just one forgetful or careless person can break access for all others
2) I have 'umask 002' in my ~/.profile. Somehow, it does not help,
because ~/.profile is not read on non-interactive SSH sessions
(to verify that, just try to do 'ssh somehost umask')

The current workaround for the problem is a cron script, which
makes 'find | xargs chmod 2775' every 5 minutes. It works, but is ugly.

Is there any better way to keep correct access rights in a shared repository?

Thanks a lot!

Re: [PATCH] git-receive-pack needs to set umask(2)

[Shawn Pearce]

Salikh Zakirov <Salikh.Zakirov@Intel.com> wrote:
> Johannes Schindelin wrote:
> > See also
> > 
> > http://thread.gmane.org/gmane.comp.version-control.git/13856/focus=13876
> 
> I've read the thread, but couldn't find a practical solution there.
>  
> > The essence of the thread: If you want to do anything useful in a non-bare 
> > repository, you are likely using other tools than git, which do not 
> > interpret core.umask or $GIT_DIR/umask.
> > 
> > If you use a bare repository, just make it shared. No need for an umask.
> 
> Could you please elaborate on what does it mean "make it shared"?
> 
> My setup: I have a bare GIT repository on a machine, where everybody can
> SSH into (with full shell access). I've assigned the repo to a special group
> where everybody belongs, and done a 'find repo.git -type d | xargs chmod 2775'
> 
> The problem: After someone pushed to the repository, the object directories 
> (i.e repo.git/objects/??)
> get created with 755 access rights, and effectively prevent everyone else from pushing
> objects starting with the same prefix.
> 
> The obvious solution to use umask 002 is not applicable, because
> 1) It does not seem practical to enforce umask 002 in everyone's rc files, 
> because just one forgetful or careless person can break access for all others
> 2) I have 'umask 002' in my ~/.profile. Somehow, it does not help,
> because ~/.profile is not read on non-interactive SSH sessions
> (to verify that, just try to do 'ssh somehost umask')
> 
> The current workaround for the problem is a cron script, which
> makes 'find | xargs chmod 2775' every 5 minutes. It works, but is ugly.
> 
> Is there any better way to keep correct access rights in a shared repository?

Try setting 'core.sharedRepository' to true:

	git repo-config core.sharedRepository true

and running your chmod script one last time.  See
Documentation/config.txt for some details on this switch.

-- 
Shawn.

RE: [PATCH] git-receive-pack needs to set umask(2)

[Zakirov, Salikh]

Thanks a lot, that works!

Shawn wrote:
>
> > Could you please elaborate on what does it mean "make it shared"?
> 
> Try setting 'core.sharedRepository' to true:
> 
> 
> 	git repo-config core.sharedRepository true
> 
> and running your chmod script one last time.  See
> Documentation/config.txt for some details on this switch.

Re: [PATCH] git-receive-pack needs to set umask(2)

[Michael Richardson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


>>>>> "Petr" == Petr Baudis <pasky@ucw.cz> writes:
    Petr> The object database is considered "append-only" unless you do
    Petr> git-prune (and you should better not let anyone do that), thus
    Petr> it's enough if you set all directories group-writable. Other

  Exactly, you have to do that. And only the owner can change the modes,
thus, unless all users have their umask set up right, someone gets toasted.
  Since the directories are created on the fly, they need to be created
with the right permissions. 

    Petr> than access the object database, the users probably only want
    Petr> to update the refs - the solution is to make refs/heads/ and
    Petr> refs/tags/ group-writable and setgid. This is also what
    Petr> git-init-db --shared (or tools like cg-admin-setuprepo) should
    Petr> already set up for you.

    Petr> So, what did break?

  Never heard of "git-init-db --shared".

> A shared repository allows users belonging to the same group to push
> into that repository. When specifying `--shared` the config variable
> "core.sharedRepository" is set to 'true' so that directories under
> `$GIT_DIR` are made group writable (and g+sx, since the git group may
> be not the primary group of all users). 

  That would seem to be the right thing.
  Seems it was added in December.

- -- 
]       ON HUMILITY: to err is human. To moo, bovine.           |  firewalls  [
]   Michael Richardson,    Xelerance Corporation, Ottawa, ON    |net architect[
] mcr@xelerance.com      http://www.sandelman.ottawa.on.ca/mcr/ |device driver[
] panic("Just another Debian GNU/Linux using, kernel hacking, security guy"); [

    "The Microsoft _Get the Facts CD_ does not work on Linux." - orospakr

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Finger me for keys

iQEVAwUBRHsbW4CLcPvd0N1lAQKa/gf+MF93+zbNnmqpysWMmYPVhW6HvU6XFyCQ
KyTfA7dxVX3tS9coSAcT73IX659umMz1MkyG7YR4ISFLlhLmdthq6l/ETueTZPVw
SgTSEU9TT2sM+gjtzy6v1wGQJAXJxYw6kJgKOFgCfyIPsb7EZWyQBmZLiNU0omnv
gkV8Ja5pJPTNHcinzzNyg8LIm0j55cS9OG9XQrXm46q+9OX+y39BoxGnz3Guzmry
yzfx1ipDuW54QCzKRyBpwt7/1LBfk/eJAH0wP9IAA4qz39+OA2yz8fTMvHDB1a6V
H18SkBENb6ZllGovu60IUgJCKy2sizGkBGUax9ec2ByAzHL1al3W3g==
=arDu
-----END PGP SIGNATURE-----

Re: [PATCH] git-receive-pack needs to set umask(2)

[Linus Torvalds]

I realize that you already found the solution (Core.SharedRepository), 
but:

On Mon, 29 May 2006, Salikh Zakirov wrote:
> 
> 2) I have 'umask 002' in my ~/.profile. Somehow, it does not help,
> because ~/.profile is not read on non-interactive SSH sessions
> (to verify that, just try to do 'ssh somehost umask')

The ".profile" thing is indeed read only for interactive tasks.

So use ".bashrc" instead.

The reason I mention that is that this has come up before: if you need to 
do things like setting PATH to point to your ~/bin directory (to use your 
own version of git rather than the system one), or if you want to set 
environment variables like GIT_COMMITTER_NAME etc, you should always use 
.bashrc, so that you get the same answers whether you log in 
interactively, or whether you just do "ssh host git-cmd".

		Linus

Re: [PATCH] git-receive-pack needs to set umask(2)

[Alex Riesen]

Linus Torvalds, Mon, May 29, 2006 19:00:42 +0200:
> 
> I realize that you already found the solution (Core.SharedRepository), 
> but:
> 
> On Mon, 29 May 2006, Salikh Zakirov wrote:
> > 
> > 2) I have 'umask 002' in my ~/.profile. Somehow, it does not help,
> > because ~/.profile is not read on non-interactive SSH sessions
> > (to verify that, just try to do 'ssh somehost umask')
> 
> The ".profile" thing is indeed read only for interactive tasks.
> 
> So use ".bashrc" instead.
> 

Will not work:

$ man bash
...
       When  an  interactive  shell  that is not a login shell is
       started, bash reads and executes commands from  ~/.bashrc,
       if  that  file exists. ...

Besides, not everyone has bash as their login shell.

Reading man sshd:

     $HOME/.ssh/rc
             If this file exists, it is run with /bin/sh after reading the
             environment files but before starting the user's shell or com­
             mand.  It must not produce any output on stdout; stderr must be
             used instead.  If X11 forwarding is in use, it will receive the
             "proto cookie" pair in its standard input (and DISPLAY in its
             environment).  The script must call xauth(1) because sshd will
             not run xauth automatically to add X11 cookies.
and

     /etc/ssh/sshrc
             Like $HOME/.ssh/rc.  This can be used to specify machine-specific
             login-time initializations globally.  This file should be
             writable only by root, and should be world-readable.


This guaranteed to work (at least for ssh).

Re: [PATCH] git-receive-pack needs to set umask(2)

[Johannes Schindelin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 984 bytes --]

Hi,

On Mon, 29 May 2006, Alex Riesen wrote:

> [...]
> Reading man sshd:
> 
>      $HOME/.ssh/rc
>              If this file exists, it is run with /bin/sh after reading the
>              environment files but before starting the user's shell or com­
>              mand.  It must not produce any output on stdout; stderr must be
>              used instead.  If X11 forwarding is in use, it will receive the
>              "proto cookie" pair in its standard input (and DISPLAY in its
>              environment).  The script must call xauth(1) because sshd will
>              not run xauth automatically to add X11 cookies.
> and
> 
>      /etc/ssh/sshrc
>              Like $HOME/.ssh/rc.  This can be used to specify machine-specific
>              login-time initializations globally.  This file should be
>              writable only by root, and should be world-readable.
> 
> 
> This guaranteed to work (at least for ssh).

But not for bash. Back to square 1.

Ciao,
Dscho