From b724448b589d60a9a7dda60cf30741048c98e199 Mon Sep 17 00:00:00 2001
From: Florian Zschocke <florian.zschocke@cycos.com>
Date: Mon, 26 Aug 2013 06:39:57 -0400
Subject: [PATCH] Fix set-gid bit clearing under Linux when effective gid is different from file gid.
---
src/main/java/com/gitblit/utils/JGitUtils.java | 16 +++
src/test/java/com/gitblit/tests/JGitUtilsTest.java | 64 ++++++++++++++++
src/test/java/com/gitblit/tests/JnaUtilsTest.java | 38 +++++++++
src/main/java/com/gitblit/utils/JnaUtils.java | 90 +++++++++++++++++++++-
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 66dbd60..2e448c3 100644
--- a/src/main/java/com/gitblit/utils/JGitUtils.java
+++ b/src/main/java/com/gitblit/utils/JGitUtils.java
@@ -403,9 +403,23 @@
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 3bf1f73..4009342 100644
--- a/src/main/java/com/gitblit/utils/JnaUtils.java
+++ b/src/main/java/com/gitblit/utils/JnaUtils.java
@@ -79,6 +79,28 @@
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 @@
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 @@
}
}
- 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 @@
*/
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 cdf698d..463c0a8 100644
--- a/src/test/java/com/gitblit/tests/JGitUtilsTest.java
+++ b/src/test/java/com/gitblit/tests/JGitUtilsTest.java
@@ -219,6 +219,70 @@
}
@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 2430b6e..25d1ccf 100644
--- a/src/test/java/com/gitblit/tests/JnaUtilsTest.java
+++ b/src/test/java/com/gitblit/tests/JnaUtilsTest.java
@@ -20,6 +20,7 @@
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;
@@ -34,6 +35,24 @@
* @author Florian Zschocke
*/
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 {
@@ -111,4 +130,23 @@
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);
+ }
+ }
+
+
}
--
Gitblit v1.9.1