From b724448b589d60a9a7dda60cf30741048c98e199 Mon Sep 17 00:00:00 2001 From: Florian Zschocke Date: Wed, 21 Aug 2013 10:39:43 +0200 Subject: [PATCH] Fix set-gid bit clearing under Linux when effective gid is different from file gid. --- .../java/com/gitblit/utils/JGitUtils.java | 16 +++- src/main/java/com/gitblit/utils/JnaUtils.java | 90 +++++++++++++++++-- .../java/com/gitblit/tests/JGitUtilsTest.java | 64 +++++++++++++ .../java/com/gitblit/tests/JnaUtilsTest.java | 38 ++++++++ 4 files changed, 201 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/gitblit/utils/JGitUtils.java b/src/main/java/com/gitblit/utils/JGitUtils.java index 66dbd60d..2e448c36 100644 --- a/src/main/java/com/gitblit/utils/JGitUtils.java +++ b/src/main/java/com/gitblit/utils/JGitUtils.java @@ -403,9 +403,23 @@ public class JGitUtils { if (! path.exists()) return -1; int perm = configShared.getPerm(); - int mode = JnaUtils.getFilemode(path); + JnaUtils.Filestat stat = JnaUtils.getFilestat(path); + if (stat == null) return -1; + int mode = stat.mode; if (mode < 0) return -1; + // Now, here is the kicker: Under Linux, chmod'ing a sgid file whose guid is different from the process' + // effective guid will reset the sgid flag of the file. Since there is no way to get the sgid flag back in + // that case, we decide to rather not touch is and getting the right permissions will have to be achieved + // in a different way, e.g. by using an appropriate umask for the Gitblit process. + if (System.getProperty("os.name").toLowerCase().startsWith("linux")) { + if ( ((mode & (JnaUtils.S_ISGID | JnaUtils.S_ISUID)) != 0) + && stat.gid != JnaUtils.getegid() ) { + LOGGER.debug("Not adjusting permissions to prevent clearing suid/sgid bits for '" + path + "'" ); + return 0; + } + } + // If the owner has no write access, delete it from group and other, too. if ((mode & JnaUtils.S_IWUSR) == 0) perm &= ~0222; // If the owner has execute access, set it for all blocks that have read access. diff --git a/src/main/java/com/gitblit/utils/JnaUtils.java b/src/main/java/com/gitblit/utils/JnaUtils.java index 3bf1f73a..4009342a 100644 --- a/src/main/java/com/gitblit/utils/JnaUtils.java +++ b/src/main/java/com/gitblit/utils/JnaUtils.java @@ -79,6 +79,28 @@ public class JnaUtils { private interface UnixCLibrary extends Library { public int chmod(String path, int mode); + public int getgid(); + public int getegid(); + } + + + public static int getgid() + { + if (isWindows()) { + throw new UnsupportedOperationException("The method JnaUtils.getgid is not supported under Windows."); + } + + return getUnixCLibrary().getgid(); + } + + + public static int getegid() + { + if (isWindows()) { + throw new UnsupportedOperationException("The method JnaUtils.getegid is not supported under Windows."); + } + + return getUnixCLibrary().getegid(); } @@ -157,21 +179,77 @@ public class JnaUtils { throw new UnsupportedOperationException("The method JnaUtils.getFilemode is not supported under Windows."); } + Filestat stat = getFilestat(path); + if ( stat == null ) return -1; + return stat.mode; + } + + + /** + * Status information of a file. + */ + public static class Filestat + { + public int mode; // file mode, permissions, type + public int uid; // user Id of owner + public int gid; // group Id of owner + + Filestat(int mode, int uid, int gid) { + this.mode = mode; this.uid = uid; this.gid = gid; + } + } + + + /** + * Get Unix file status information for a file. + * + * This method is only implemented for OSes of the Unix family. It returns file status + * information for a file. Currently this is the file mode, the user id and group id of the owner. + * + * @param path + * File/directory to get the file status from. + * @return Upon successful completion, a Filestat object containing the file information is returned. + * Otherwise, null is returned. + */ + public static Filestat getFilestat(File path) + { + return getFilestat(path.getAbsolutePath()); + } + + + /** + * Get Unix file status information for a file. + * + * This method is only implemented for OSes of the Unix family. It returns file status + * information for a file. Currently this is the file mode, the user id and group id of the owner. + * + * @param path + * Path to a file/directory to get the file status from. + * @return Upon successful completion, a Filestat object containing the file information is returned. + * Otherwise, null is returned. + */ + public static Filestat getFilestat(String path) + { + if (isWindows()) { + throw new UnsupportedOperationException("The method JnaUtils.getFilestat is not supported under Windows."); + } + int mode = 0; // Use a Runtime, because implementing stat() via JNA is just too much trouble. + // This could be done with the 'stat' command, too. But that may have a shell specific implementation, so we use 'ls' instead. String lsLine = runProcessLs(path); if (lsLine == null) { LOGGER.debug("Could not get file information for path " + path); - return -1; + return null; } - Pattern p = Pattern.compile("^(([-bcdlsp])([-r][-w][-xSs])([-r][-w][-xSs])([-r][-w][-xTt])) "); + Pattern p = Pattern.compile("^(([-bcdlspCDMnP?])([-r][-w][-xSs])([-r][-w][-xSs])([-r][-w][-xTt]))[@+.]? +[0-9]+ +([0-9]+) +([0-9]+) "); Matcher m = p.matcher(lsLine); if ( !m.lookingAt() ) { LOGGER.debug("Could not parse valid file mode information for path " + path); - return -1; + return null; } // Parse mode string to mode bits @@ -227,12 +305,12 @@ public class JnaUtils { } } - return mode; + return new Filestat(mode, Integer.parseInt(m.group(6)), Integer.parseInt(m.group(7))); } /** - * Run the unix command 'ls -ldO' on a single file and return the resulting output line. + * Run the unix command 'ls -ldn' on a single file and return the resulting output line. * * @param path * Path to a single file or directory. @@ -240,7 +318,7 @@ public class JnaUtils { */ private static String runProcessLs(String path) { - String cmd = "ls -ld " + path; + String cmd = "ls -ldn " + path; Runtime rt = Runtime.getRuntime(); Process pr = null; InputStreamReader ir = null; diff --git a/src/test/java/com/gitblit/tests/JGitUtilsTest.java b/src/test/java/com/gitblit/tests/JGitUtilsTest.java index cdf698d6..463c0a84 100644 --- a/src/test/java/com/gitblit/tests/JGitUtilsTest.java +++ b/src/test/java/com/gitblit/tests/JGitUtilsTest.java @@ -218,6 +218,70 @@ public class JGitUtilsTest { } } + @Test + public void testCreateRepositorySharedSgidParent() throws Exception { + if (! JnaUtils.isWindows()) { + String repositoryAll = "NewTestRepositoryAll.git"; + String repositoryUmask = "NewTestRepositoryUmask.git"; + String sgidParent = "sgid"; + + File parent = new File(GitBlitSuite.REPOSITORIES, sgidParent); + File folder = null; + boolean parentExisted = parent.exists(); + try { + if (!parentExisted) { + assertTrue("Could not create SGID parent folder.", parent.mkdir()); + } + int mode = JnaUtils.getFilemode(parent); + assertTrue(mode > 0); + assertEquals(0, JnaUtils.setFilemode(parent, mode | JnaUtils.S_ISGID | JnaUtils.S_IWGRP)); + + Repository repository = JGitUtils.createRepository(parent, repositoryAll, "all"); + folder = FileKey.resolve(new File(parent, repositoryAll), FS.DETECTED); + assertNotNull(repository); + + assertEquals("2", repository.getConfig().getString("core", null, "sharedRepository")); + + assertTrue(folder.exists()); + mode = JnaUtils.getFilemode(folder); + assertEquals(JnaUtils.S_ISGID, mode & JnaUtils.S_ISGID); + + mode = JnaUtils.getFilemode(folder.getAbsolutePath() + "/HEAD"); + assertEquals(JnaUtils.S_IRGRP | JnaUtils.S_IWGRP, mode & JnaUtils.S_IRWXG); + assertEquals(JnaUtils.S_IROTH, mode & JnaUtils.S_IRWXO); + + mode = JnaUtils.getFilemode(folder.getAbsolutePath() + "/config"); + assertEquals(JnaUtils.S_IRGRP | JnaUtils.S_IWGRP, mode & JnaUtils.S_IRWXG); + assertEquals(JnaUtils.S_IROTH, mode & JnaUtils.S_IRWXO); + + repository.close(); + RepositoryCache.close(repository); + + + + repository = JGitUtils.createRepository(parent, repositoryUmask, "umask"); + folder = FileKey.resolve(new File(parent, repositoryUmask), FS.DETECTED); + assertNotNull(repository); + + assertEquals(null, repository.getConfig().getString("core", null, "sharedRepository")); + + assertTrue(folder.exists()); + mode = JnaUtils.getFilemode(folder); + assertEquals(JnaUtils.S_ISGID, mode & JnaUtils.S_ISGID); + + repository.close(); + RepositoryCache.close(repository); + } + finally { + FileUtils.delete(new File(parent, repositoryAll), FileUtils.RECURSIVE | FileUtils.IGNORE_ERRORS); + FileUtils.delete(new File(parent, repositoryUmask), FileUtils.RECURSIVE | FileUtils.IGNORE_ERRORS); + if (!parentExisted) { + FileUtils.delete(parent, FileUtils.RECURSIVE | FileUtils.IGNORE_ERRORS); + } + } + } + } + @Test public void testRefs() throws Exception { Repository repository = GitBlitSuite.getJGitRepository(); diff --git a/src/test/java/com/gitblit/tests/JnaUtilsTest.java b/src/test/java/com/gitblit/tests/JnaUtilsTest.java index 2430b6ea..25d1ccf5 100644 --- a/src/test/java/com/gitblit/tests/JnaUtilsTest.java +++ b/src/test/java/com/gitblit/tests/JnaUtilsTest.java @@ -20,6 +20,7 @@ import com.gitblit.utils.JnaUtils; import java.io.File; import java.io.IOException; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import org.apache.commons.io.FileUtils; @@ -35,6 +36,24 @@ import org.junit.Test; */ public class JnaUtilsTest { + @Test + public void testGetgid() { + if (JnaUtils.isWindows()) { + try { + JnaUtils.getFilemode(GitBlitSuite.REPOSITORIES); + } catch(UnsupportedOperationException e) {} + } + else { + int gid = JnaUtils.getgid(); + assertTrue(gid >= 0); + int egid = JnaUtils.getegid(); + assertTrue(egid >= 0); + assertTrue("Really? You're running unit tests as root?!", gid > 0); + System.out.println("gid: " + gid + " egid: " + egid); + } + } + + @Test public void testGetFilemode() throws IOException { if (JnaUtils.isWindows()) { @@ -111,4 +130,23 @@ public class JnaUtilsTest { FileUtils.deleteDirectory(repository.getDirectory()); } } + + + @Test + public void testGetFilestat() { + if (JnaUtils.isWindows()) { + try { + JnaUtils.getFilemode(GitBlitSuite.REPOSITORIES); + } catch(UnsupportedOperationException e) {} + } + else { + JnaUtils.Filestat stat = JnaUtils.getFilestat(GitBlitSuite.REPOSITORIES); + assertNotNull(stat); + assertTrue(stat.mode > 0); + assertTrue(stat.uid > 0); + assertTrue(stat.gid > 0); + } + } + + } -- 2.39.5