Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.

We used to create 0600 files, and then use chmod to set the group and
other permission bits to the umask. This usually has the same effect
as a normal file creation with a umask.

But in the presence of ACLs, the group permission plays the role of
the ACL mask: the "g" bits of newly created files are chosen according
to default ACL mask of the directory, not according to the umask, and
doing a chmod() on these "g" bits affect the ACL's mask instead of
actual group permission.

In other words, creating files with 0600 and then doing a chmod to the
umask creates files which are unreadable by users allowed in the
default ACL. To create the files without breaking ACLs, we let the
umask do it's job at the file's creation time, and get rid of the
later chmod.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index e1d3adf405bb6ac842a3415e0461b4772396060d..539e75d56f7a33fdb1971b51ee0681c43e9662b0 100644 (file)
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -464,9 +464,6 @@ static int write_one(struct sha1file *f,
        return 1;
 }
 
        return 1;
 }
 

-/* forward declaration for write_pack_file */
-static int adjust_perm(const char *path, mode_t mode);
-

 static void write_pack_file(void)
 {
        uint32_t i = 0, j;
 static void write_pack_file(void)
 {
        uint32_t i = 0, j;


@@ -523,21 +520,17 @@ static void write_pack_file(void)
                }
 
                if (!pack_to_stdout) {
                }
 
                if (!pack_to_stdout) {

-                       mode_t mode = umask(0);

                        struct stat st;
                        const char *idx_tmp_name;
                        char tmpname[PATH_MAX];
 
                        struct stat st;
                        const char *idx_tmp_name;
                        char tmpname[PATH_MAX];
 

-                       umask(mode);
-                       mode = 0444 & ~mode;
-

                        idx_tmp_name = write_idx_file(NULL, written_list,
                                                      nr_written, sha1);
 
                        snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
                                 base_name, sha1_to_hex(sha1));
                        free_pack_by_name(tmpname);
                        idx_tmp_name = write_idx_file(NULL, written_list,
                                                      nr_written, sha1);
 
                        snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
                                 base_name, sha1_to_hex(sha1));
                        free_pack_by_name(tmpname);

-                       if (adjust_perm(pack_tmp_name, mode))
+                       if (adjust_shared_perm(pack_tmp_name))

                                die_errno("unable to make temporary pack file readable");
                        if (rename(pack_tmp_name, tmpname))
                                die_errno("unable to rename temporary pack file");
                                die_errno("unable to make temporary pack file readable");
                        if (rename(pack_tmp_name, tmpname))
                                die_errno("unable to rename temporary pack file");


@@ -565,7 +558,7 @@ static void write_pack_file(void)
 
                        snprintf(tmpname, sizeof(tmpname), "%s-%s.idx",
                                 base_name, sha1_to_hex(sha1));
 
                        snprintf(tmpname, sizeof(tmpname), "%s-%s.idx",
                                 base_name, sha1_to_hex(sha1));

-                       if (adjust_perm(idx_tmp_name, mode))
+                       if (adjust_shared_perm(idx_tmp_name))

                                die_errno("unable to make temporary index file readable");
                        if (rename(idx_tmp_name, tmpname))
                                die_errno("unable to rename temporary index file");
                                die_errno("unable to make temporary index file readable");
                        if (rename(idx_tmp_name, tmpname))
                                die_errno("unable to rename temporary index file");


@@ -2125,13 +2118,6 @@ static void get_object_list(int ac, const char **av)
                loosen_unused_packed_objects(&revs);
 }
 
                loosen_unused_packed_objects(&revs);
 }
 

-static int adjust_perm(const char *path, mode_t mode)
-{
-       if (chmod(path, mode))
-               return -1;
-       return adjust_shared_perm(path);
-}
-

 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
        int use_internal_rev_list = 0;
 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
        int use_internal_rev_list = 0;

diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 07dd6af99cde58e2df0b777f68a347e7068823bb..8472dbb44a500b1e4af6f76fff5f1ba79782621e 100755 (executable)
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -59,7 +59,7 @@ test_expect_failure 'Objects creation does not break ACLs with restrictive umask
        check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
 '
 
        check_perms_and_acl .git/objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391
 '
 

-test_expect_failure 'git gc does not break ACLs with restrictive umask' '
+test_expect_success 'git gc does not break ACLs with restrictive umask' '

        git gc &&
        check_perms_and_acl .git/objects/pack/*.pack
 '
        git gc &&
        check_perms_and_acl .git/objects/pack/*.pack
 '

diff --git a/wrapper.c b/wrapper.c
index 673762fde910b2a1740194461f4b0cdd15ce49ca..9c71b21242773f52ca560d7e5e5e52123674d334 100644 (file)
--- a/wrapper.c
+++ b/wrapper.c
@@ -277,10 +277,14 @@ int git_inflate(z_streamp strm, int flush)
 int odb_mkstemp(char *template, size_t limit, const char *pattern)
 {
        int fd;
 int odb_mkstemp(char *template, size_t limit, const char *pattern)
 {
        int fd;

-
+       /*
+        * we let the umask do its job, don't try to be more
+        * restrictive except to remove write permission.
+        */
+       int mode = 0444;

        snprintf(template, limit, "%s/%s",
                 get_object_directory(), pattern);
        snprintf(template, limit, "%s/%s",
                 get_object_directory(), pattern);

-       fd = mkstemp(template);
+       fd = git_mkstemp_mode(template, mode);

        if (0 <= fd)
                return fd;
 
        if (0 <= fd)
                return fd;
 


@@ -289,7 +293,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)
        snprintf(template, limit, "%s/%s",
                 get_object_directory(), pattern);
        safe_create_leading_directories(template);
        snprintf(template, limit, "%s/%s",
                 get_object_directory(), pattern);
        safe_create_leading_directories(template);

-       return xmkstemp(template);
+       return xmkstemp_mode(template, mode);

 }
 
 int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)
 }
 
 int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)