Fix error occurring when SecurityManager is enabled
It's expected that jgit should work without native git installation.
In such case Security Manager can be configured to deny access to the
files outside of git repository. JGit tries to find cygwin
installation. If Security manager restricts access to some folders
in PATH, it should be considered that those folders are absent
for jgit.
Also JGit tries to detect if symbolic links are supported by OS. If
security manager forbids creation of symlinks, it should be assumed
that symlinks aren't supported.
Bug: 550115
Change-Id: Ic4b243cada604bc1090db6cc1cfd74f0fa324b98
Signed-off-by: Nail Samatov <sanail@yandex.ru>
diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java
new file mode 100644
index 0000000..926a195
--- /dev/null
+++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2019 Nail Samatov <sanail@yandex.ru>
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.junit;
+
+import static java.lang.ClassLoader.getSystemClassLoader;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+
+import org.junit.runners.BlockJUnit4ClassRunner;
+import org.junit.runners.model.InitializationError;
+
+/**
+ * This class is used when it's required to load jgit classes in separate
+ * classloader for each test class. It can be needed to isolate static field
+ * initialization between separate tests.
+ */
+public class SeparateClassloaderTestRunner extends BlockJUnit4ClassRunner {
+
+ /**
+ * Creates a SeparateClassloaderTestRunner to run {@code klass}.
+ *
+ * @param klass
+ * test class to run.
+ * @throws InitializationError
+ * if the test class is malformed or can't be found.
+ */
+ public SeparateClassloaderTestRunner(Class<?> klass)
+ throws InitializationError {
+ super(loadNewClass(klass));
+ }
+
+ private static Class<?> loadNewClass(Class<?> klass)
+ throws InitializationError {
+ try {
+ URL[] urls = ((URLClassLoader) getSystemClassLoader()).getURLs();
+ ClassLoader testClassLoader = new URLClassLoader(urls) {
+
+ @Override
+ public Class<?> loadClass(String name)
+ throws ClassNotFoundException {
+ if (name.startsWith("org.eclipse.jgit.")) {
+ return super.findClass(name);
+ }
+
+ return super.loadClass(name);
+ }
+ };
+ return Class.forName(klass.getName(), true, testClassLoader);
+ } catch (ClassNotFoundException e) {
+ throw new InitializationError(e);
+ }
+ }
+}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java
new file mode 100644
index 0000000..62d3530
--- /dev/null
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java
@@ -0,0 +1,206 @@
+/*
+ * Copyright (C) 2019 Nail Samatov <sanail@yandex.ru>
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior
+ * written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.eclipse.jgit.api;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.FilePermission;
+import java.io.IOException;
+import java.lang.reflect.ReflectPermission;
+import java.nio.file.Files;
+import java.security.Permission;
+import java.security.SecurityPermission;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.PropertyPermission;
+import java.util.logging.LoggingPermission;
+
+import javax.security.auth.AuthPermission;
+
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.junit.JGitTestUtil;
+import org.eclipse.jgit.junit.MockSystemReader;
+import org.eclipse.jgit.junit.SeparateClassloaderTestRunner;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.util.FileUtils;
+import org.eclipse.jgit.util.SystemReader;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * <p>
+ * Tests if jgit works if SecurityManager is enabled.
+ * </p>
+ *
+ * <p>
+ * Note: JGit's classes shouldn't be used before SecurityManager is configured.
+ * If you use some JGit's class before SecurityManager is replaced then part of
+ * the code can be invoked outside of our custom SecurityManager and this test
+ * becomes useless.
+ * </p>
+ *
+ * <p>
+ * For example the class {@link org.eclipse.jgit.util.FS} is used widely in jgit
+ * sources. It contains DETECTED static field. At the first usage of the class
+ * FS the field DETECTED is initialized and during initialization many system
+ * operations that SecurityManager can forbid are invoked.
+ * </p>
+ *
+ * <p>
+ * For this reason this test doesn't extend LocalDiskRepositoryTestCase (it uses
+ * JGit's classes in setUp() method) and other JGit's utility classes. It's done
+ * to affect SecurityManager as less as possible.
+ * </p>
+ *
+ * <p>
+ * We use SeparateClassloaderTestRunner to isolate FS.DETECTED field
+ * initialization between different tests run.
+ * </p>
+ */
+@RunWith(SeparateClassloaderTestRunner.class)
+public class SecurityManagerTest {
+ private File root;
+
+ private SecurityManager originalSecurityManager;
+
+ private List<Permission> permissions = new ArrayList<>();
+
+ @Before
+ public void setUp() throws Exception {
+ // Create working directory
+ SystemReader.setInstance(new MockSystemReader());
+ root = Files.createTempDirectory("jgit-security").toFile();
+
+ // Add system permissions
+ permissions.add(new RuntimePermission("*"));
+ permissions.add(new SecurityPermission("*"));
+ permissions.add(new AuthPermission("*"));
+ permissions.add(new ReflectPermission("*"));
+ permissions.add(new PropertyPermission("*", "read,write"));
+ permissions.add(new LoggingPermission("control", null));
+
+ permissions.add(new FilePermission(
+ System.getProperty("java.home") + "/-", "read"));
+
+ String tempDir = System.getProperty("java.io.tmpdir");
+ permissions.add(new FilePermission(tempDir, "read,write,delete"));
+ permissions
+ .add(new FilePermission(tempDir + "/-", "read,write,delete"));
+
+ // Add permissions to dependent jar files.
+ String classPath = System.getProperty("java.class.path");
+ if (classPath != null) {
+ for (String path : classPath.split(File.pathSeparator)) {
+ permissions.add(new FilePermission(path, "read"));
+ }
+ }
+ // Add permissions to jgit class files.
+ String jgitSourcesRoot = new File(System.getProperty("user.dir"))
+ .getParent();
+ permissions.add(new FilePermission(jgitSourcesRoot + "/-", "read"));
+
+ // Add permissions to working dir for jgit. Our git repositories will be
+ // initialized and cloned here.
+ permissions.add(new FilePermission(root.getPath() + "/-",
+ "read,write,delete,execute"));
+
+ // Replace Security Manager
+ originalSecurityManager = System.getSecurityManager();
+ System.setSecurityManager(new SecurityManager() {
+
+ @Override
+ public void checkPermission(Permission requested) {
+ for (Permission permission : permissions) {
+ if (permission.implies(requested)) {
+ return;
+ }
+ }
+
+ super.checkPermission(requested);
+ }
+ });
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ System.setSecurityManager(originalSecurityManager);
+
+ // Note: don't use this method before security manager is replaced in
+ // setUp() method. The method uses FS.DETECTED internally and can affect
+ // the test.
+ FileUtils.delete(root, FileUtils.RECURSIVE | FileUtils.RETRY);
+ }
+
+ @Test
+ public void testInitAndClone() throws IOException, GitAPIException {
+ File remote = new File(root, "remote");
+ File local = new File(root, "local");
+
+ try (Git git = Git.init().setDirectory(remote).call()) {
+ JGitTestUtil.write(new File(remote, "hello.txt"), "Hello world!");
+ git.add().addFilepattern(".").call();
+ git.commit().setMessage("Initial commit").call();
+ }
+
+ try (Git git = Git.cloneRepository().setURI(remote.toURI().toString())
+ .setDirectory(local).call()) {
+ assertTrue(new File(local, ".git").exists());
+
+ JGitTestUtil.write(new File(local, "hi.txt"), "Hi!");
+ git.add().addFilepattern(".").call();
+ RevCommit commit1 = git.commit().setMessage("Commit on local repo")
+ .call();
+ assertEquals("Commit on local repo", commit1.getFullMessage());
+ assertNotNull(TreeWalk.forPath(git.getRepository(), "hello.txt",
+ commit1.getTree()));
+ }
+
+ }
+
+}
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 2a2ee8d..5a4d9bb 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -587,6 +587,8 @@
readerIsRequired=Reader is required
readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0}
readLastModifiedFailed=Reading lastModified of {0} failed
+readPipeIsNotAllowed=FS.readPipe() isn't allowed for command ''{0}''. Working directory: ''{1}''.
+readPipeIsNotAllowedRequiredPermission=FS.readPipe() isn't allowed for command ''{0}''. Working directory: ''{1}''. Required permission: {2}.
readTimedOut=Read timed out after {0} ms
receivePackObjectTooLarge1=Object too large, rejecting the pack. Max object size limit is {0} bytes.
receivePackObjectTooLarge2=Object too large ({0} bytes), rejecting the pack. Max object size limit is {1} bytes.
@@ -662,6 +664,7 @@
similarityScoreMustBeWithinBounds=Similarity score must be between 0 and 100.
sizeExceeds2GB=Path {0} size {1} exceeds 2 GiB limit.
skipMustBeNonNegative=skip must be >= 0
+skipNotAccessiblePath=The path ''{0}'' isn't accessible. Skip it.
smartHTTPPushDisabled=smart HTTP push disabled
sourceDestinationMustMatch=Source/Destination must match.
sourceIsNotAWildcard=Source is not a wildcard.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 649f777..b80b749 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -648,6 +648,8 @@
/***/ public String readerIsRequired;
/***/ public String readingObjectsFromLocalRepositoryFailed;
/***/ public String readLastModifiedFailed;
+ /***/ public String readPipeIsNotAllowed;
+ /***/ public String readPipeIsNotAllowedRequiredPermission;
/***/ public String readTimedOut;
/***/ public String receivePackObjectTooLarge1;
/***/ public String receivePackObjectTooLarge2;
@@ -723,6 +725,7 @@
/***/ public String similarityScoreMustBeWithinBounds;
/***/ public String sizeExceeds2GB;
/***/ public String skipMustBeNonNegative;
+ /***/ public String skipNotAccessiblePath;
/***/ public String smartHTTPPushDisabled;
/***/ public String sourceDestinationMustMatch;
/***/ public String sourceIsNotAWildcard;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
index 9030501..2951929 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
@@ -64,6 +64,7 @@
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
+import java.security.AccessControlException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.text.MessageFormat;
@@ -122,6 +123,8 @@
*/
protected static final Entry[] NO_ENTRIES = {};
+ private volatile Boolean supportSymlinks;
+
/**
* This class creates FS instances. It will be overridden by a Java7 variant
* if such can be detected in {@link #detect(Boolean)}.
@@ -276,15 +279,19 @@
* @return FileStoreAttributes for the given path.
*/
public static FileStoreAttributes get(Path path) {
- path = path.toAbsolutePath();
- Path dir = Files.isDirectory(path) ? path : path.getParent();
- FileStoreAttributes cached = attrCacheByPath.get(dir);
- if (cached != null) {
- return cached;
+ try {
+ path = path.toAbsolutePath();
+ Path dir = Files.isDirectory(path) ? path : path.getParent();
+ FileStoreAttributes cached = attrCacheByPath.get(dir);
+ if (cached != null) {
+ return cached;
+ }
+ FileStoreAttributes attrs = getFileStoreAttributes(dir);
+ attrCacheByPath.put(dir, attrs);
+ return attrs;
+ } catch (SecurityException e) {
+ return FALLBACK_FILESTORE_ATTRIBUTES;
}
- FileStoreAttributes attrs = getFileStoreAttributes(dir);
- attrCacheByPath.put(dir, attrs);
- return attrs;
}
private static FileStoreAttributes getFileStoreAttributes(Path dir) {
@@ -813,7 +820,32 @@
* @since 3.0
*/
public boolean supportsSymlinks() {
- return false;
+ if (supportSymlinks == null) {
+ detectSymlinkSupport();
+ }
+ return Boolean.TRUE.equals(supportSymlinks);
+ }
+
+ private void detectSymlinkSupport() {
+ File tempFile = null;
+ try {
+ tempFile = File.createTempFile("tempsymlinktarget", ""); //$NON-NLS-1$ //$NON-NLS-2$
+ File linkName = new File(tempFile.getParentFile(), "tempsymlink"); //$NON-NLS-1$
+ createSymLink(linkName, tempFile.getPath());
+ supportSymlinks = Boolean.TRUE;
+ linkName.delete();
+ } catch (IOException | UnsupportedOperationException | SecurityException
+ | InternalError e) {
+ supportSymlinks = Boolean.FALSE;
+ } finally {
+ if (tempFile != null) {
+ try {
+ FileUtils.delete(tempFile);
+ } catch (IOException e) {
+ throw new RuntimeException(e); // panic
+ }
+ }
+ }
}
/**
@@ -1067,9 +1099,16 @@
for (String p : path.split(File.pathSeparator)) {
for (String command : lookFor) {
- final File e = new File(p, command);
- if (e.isFile())
- return e.getAbsoluteFile();
+ final File file = new File(p, command);
+ try {
+ if (file.isFile()) {
+ return file.getAbsoluteFile();
+ }
+ } catch (SecurityException e) {
+ LOG.warn(MessageFormat.format(
+ JGitText.get().skipNotAccessiblePath,
+ file.getPath()));
+ }
}
}
return null;
@@ -1172,6 +1211,13 @@
}
} catch (IOException e) {
LOG.error("Caught exception in FS.readPipe()", e); //$NON-NLS-1$
+ } catch (AccessControlException e) {
+ LOG.warn(MessageFormat.format(
+ JGitText.get().readPipeIsNotAllowedRequiredPermission,
+ command, dir, e.getPermission()));
+ } catch (SecurityException e) {
+ LOG.warn(MessageFormat.format(JGitText.get().readPipeIsNotAllowed,
+ command, dir));
}
if (debug) {
LOG.debug("readpipe returns null"); //$NON-NLS-1$
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
index a485389..6a1eef2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
@@ -287,12 +287,6 @@
/** {@inheritDoc} */
@Override
- public boolean supportsSymlinks() {
- return true;
- }
-
- /** {@inheritDoc} */
- @Override
public void setHidden(File path, boolean hidden) throws IOException {
// no action on POSIX
}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
index 7fe80bb..1e64a38 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java
@@ -74,8 +74,6 @@
public class FS_Win32 extends FS {
private final static Logger LOG = LoggerFactory.getLogger(FS_Win32.class);
- private volatile Boolean supportSymlinks;
-
/**
* Constructor
*/
@@ -239,37 +237,6 @@
/** {@inheritDoc} */
@Override
- public boolean supportsSymlinks() {
- if (supportSymlinks == null) {
- detectSymlinkSupport();
- }
- return Boolean.TRUE.equals(supportSymlinks);
- }
-
- private void detectSymlinkSupport() {
- File tempFile = null;
- try {
- tempFile = File.createTempFile("tempsymlinktarget", ""); //$NON-NLS-1$ //$NON-NLS-2$
- File linkName = new File(tempFile.getParentFile(), "tempsymlink"); //$NON-NLS-1$
- createSymLink(linkName, tempFile.getPath());
- supportSymlinks = Boolean.TRUE;
- linkName.delete();
- } catch (IOException | UnsupportedOperationException
- | InternalError e) {
- supportSymlinks = Boolean.FALSE;
- } finally {
- if (tempFile != null) {
- try {
- FileUtils.delete(tempFile);
- } catch (IOException e) {
- throw new RuntimeException(e); // panic
- }
- }
- }
- }
-
- /** {@inheritDoc} */
- @Override
public Attributes getAttributes(File path) {
return FileUtils.getFileAttributesBasic(this, path);
}
