Fix set-gid bit clearing under Linux when effective gid is different from file gid.

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
@@ -219,6 +219,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();

 		Map<ObjectId, List<RefModel>> map = JGitUtils.getAllRefs(repository);

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;
@@ -36,6 +37,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()) {
 			try {
@@ -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);
+		}
+	}
+
+
 }