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>

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);
+		}
+	}
+}

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()));
+		}
+
+	}
+
+}

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -587,6 +587,8 @@ readFileStoreAttributesFailed=Reading FileStore attributes from user config fail
 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 @@ signingNotSupportedOnTag=Signing isn't supported on tag operations yet.
 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.

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -648,6 +648,8 @@ public class JGitText extends TranslationBundle {
 	/***/ 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 class JGitText extends TranslationBundle {
 	/***/ public String similarityScoreMustBeWithinBounds;
 	/***/ public String sizeExceeds2GB;
 	/***/ public String skipMustBeNonNegative;
+	/***/ public String skipNotAccessiblePath;
 	/***/ public String smartHTTPPushDisabled;
 	/***/ public String sourceDestinationMustMatch;
 	/***/ public String sourceIsNotAWildcard;

org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java

@@ -64,6 +64,7 @@ import java.nio.file.Files;
 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 @@ public abstract class FS {
 	 */
 	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,6 +279,7 @@ public abstract class FS {
 		 * @return FileStoreAttributes for the given path.
 		 */
 		public static FileStoreAttributes get(Path path) {
+			try {
 				path = path.toAbsolutePath();
 				Path dir = Files.isDirectory(path) ? path : path.getParent();
 				FileStoreAttributes cached = attrCacheByPath.get(dir);
@@ -285,6 +289,9 @@ public abstract class FS {
 				FileStoreAttributes attrs = getFileStoreAttributes(dir);
 				attrCacheByPath.put(dir, attrs);
 				return attrs;
+			} catch (SecurityException e) {
+				return FALLBACK_FILESTORE_ATTRIBUTES;
+			}
 		}
 
 		private static FileStoreAttributes getFileStoreAttributes(Path dir) {
@@ -813,7 +820,32 @@ public abstract class FS {
 	 * @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 @@ public abstract class FS {
 
 		for (String p : path.split(File.pathSeparator)) {
 			for (String command : lookFor) {
-				final File e = new File(p, command);
+				final File file = new File(p, command);
-				if (e.isFile())
+				try {
-					return e.getAbsoluteFile();
+					if (file.isFile()) {
+						return file.getAbsoluteFile();
+					}
+				} catch (SecurityException e) {
+					LOG.warn(MessageFormat.format(
+							JGitText.get().skipNotAccessiblePath,
+							file.getPath()));
+				}
 			}
 		}
 		return null;
@@ -1172,6 +1211,13 @@ public abstract class FS {
 			}
 		} 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$

org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java

@@ -285,12 +285,6 @@ public class FS_POSIX extends FS {
 		return false;
 	}
 
-	/** {@inheritDoc} */
-	@Override
-	public boolean supportsSymlinks() {
-		return true;
-	}
-
 	/** {@inheritDoc} */
 	@Override
 	public void setHidden(File path, boolean hidden) throws IOException {

org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java

@@ -74,8 +74,6 @@ import org.slf4j.LoggerFactory;
 public class FS_Win32 extends FS {
 	private final static Logger LOG = LoggerFactory.getLogger(FS_Win32.class);
 
-	private volatile Boolean supportSymlinks;
-
 	/**
 	 * Constructor
 	 */
@@ -237,37 +235,6 @@ public class FS_Win32 extends FS {
 		return proc;
 	}
 
-	/** {@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) {