Browse Source

Merge "Fix error occurring when SecurityManager is enabled"

stable-5.5
Matthias Sohn 5 years ago committed by Gerrit Code Review @ Eclipse.org
parent
commit
c1873b0604
  1. 94
      org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/SeparateClassloaderTestRunner.java
  2. 206
      org.eclipse.jgit.test/tst/org/eclipse/jgit/api/SecurityManagerTest.java
  3. 3
      org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
  4. 3
      org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
  5. 70
      org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java
  6. 6
      org.eclipse.jgit/src/org/eclipse/jgit/util/FS_POSIX.java
  7. 33
      org.eclipse.jgit/src/org/eclipse/jgit/util/FS_Win32.java

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

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

3
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 readerIsRequired=Reader is required
readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0} readingObjectsFromLocalRepositoryFailed=reading objects from local repository failed: {0}
readLastModifiedFailed=Reading lastModified of {0} failed 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 readTimedOut=Read timed out after {0} ms
receivePackObjectTooLarge1=Object too large, rejecting the pack. Max object size limit is {0} bytes. 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. 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. similarityScoreMustBeWithinBounds=Similarity score must be between 0 and 100.
sizeExceeds2GB=Path {0} size {1} exceeds 2 GiB limit. sizeExceeds2GB=Path {0} size {1} exceeds 2 GiB limit.
skipMustBeNonNegative=skip must be >= 0 skipMustBeNonNegative=skip must be >= 0
skipNotAccessiblePath=The path ''{0}'' isn't accessible. Skip it.
smartHTTPPushDisabled=smart HTTP push disabled smartHTTPPushDisabled=smart HTTP push disabled
sourceDestinationMustMatch=Source/Destination must match. sourceDestinationMustMatch=Source/Destination must match.
sourceIsNotAWildcard=Source is not a wildcard. sourceIsNotAWildcard=Source is not a wildcard.

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

@ -648,6 +648,8 @@ public class JGitText extends TranslationBundle {
/***/ public String readerIsRequired; /***/ public String readerIsRequired;
/***/ public String readingObjectsFromLocalRepositoryFailed; /***/ public String readingObjectsFromLocalRepositoryFailed;
/***/ public String readLastModifiedFailed; /***/ public String readLastModifiedFailed;
/***/ public String readPipeIsNotAllowed;
/***/ public String readPipeIsNotAllowedRequiredPermission;
/***/ public String readTimedOut; /***/ public String readTimedOut;
/***/ public String receivePackObjectTooLarge1; /***/ public String receivePackObjectTooLarge1;
/***/ public String receivePackObjectTooLarge2; /***/ public String receivePackObjectTooLarge2;
@ -723,6 +725,7 @@ public class JGitText extends TranslationBundle {
/***/ public String similarityScoreMustBeWithinBounds; /***/ public String similarityScoreMustBeWithinBounds;
/***/ public String sizeExceeds2GB; /***/ public String sizeExceeds2GB;
/***/ public String skipMustBeNonNegative; /***/ public String skipMustBeNonNegative;
/***/ public String skipNotAccessiblePath;
/***/ public String smartHTTPPushDisabled; /***/ public String smartHTTPPushDisabled;
/***/ public String sourceDestinationMustMatch; /***/ public String sourceDestinationMustMatch;
/***/ public String sourceIsNotAWildcard; /***/ public String sourceIsNotAWildcard;

70
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.Path;
import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime; import java.nio.file.attribute.FileTime;
import java.security.AccessControlException;
import java.security.AccessController; import java.security.AccessController;
import java.security.PrivilegedAction; import java.security.PrivilegedAction;
import java.text.MessageFormat; import java.text.MessageFormat;
@ -122,6 +123,8 @@ public abstract class FS {
*/ */
protected static final Entry[] NO_ENTRIES = {}; protected static final Entry[] NO_ENTRIES = {};
private volatile Boolean supportSymlinks;
/** /**
* This class creates FS instances. It will be overridden by a Java7 variant * This class creates FS instances. It will be overridden by a Java7 variant
* if such can be detected in {@link #detect(Boolean)}. * if such can be detected in {@link #detect(Boolean)}.
@ -276,15 +279,19 @@ public abstract class FS {
* @return FileStoreAttributes for the given path. * @return FileStoreAttributes for the given path.
*/ */
public static FileStoreAttributes get(Path path) { public static FileStoreAttributes get(Path path) {
path = path.toAbsolutePath(); try {
Path dir = Files.isDirectory(path) ? path : path.getParent(); path = path.toAbsolutePath();
FileStoreAttributes cached = attrCacheByPath.get(dir); Path dir = Files.isDirectory(path) ? path : path.getParent();
if (cached != null) { FileStoreAttributes cached = attrCacheByPath.get(dir);
return cached; 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) { private static FileStoreAttributes getFileStoreAttributes(Path dir) {
@ -813,7 +820,32 @@ public abstract class FS {
* @since 3.0 * @since 3.0
*/ */
public boolean supportsSymlinks() { 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 p : path.split(File.pathSeparator)) {
for (String command : lookFor) { 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; return null;
@ -1172,6 +1211,13 @@ public abstract class FS {
} }
} catch (IOException e) { } catch (IOException e) {
LOG.error("Caught exception in FS.readPipe()", e); //$NON-NLS-1$ 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) { if (debug) {
LOG.debug("readpipe returns null"); //$NON-NLS-1$ LOG.debug("readpipe returns null"); //$NON-NLS-1$

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

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

33
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 { public class FS_Win32 extends FS {
private final static Logger LOG = LoggerFactory.getLogger(FS_Win32.class); private final static Logger LOG = LoggerFactory.getLogger(FS_Win32.class);
private volatile Boolean supportSymlinks;
/** /**
* Constructor * Constructor
*/ */
@ -237,37 +235,6 @@ public class FS_Win32 extends FS {
return proc; 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} */ /** {@inheritDoc} */
@Override @Override
public Attributes getAttributes(File path) { public Attributes getAttributes(File path) {

Loading…
Cancel
Save