You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2014/08/14 12:50:50 UTC
[1/3] git commit: BROOKLYN-15: enforce brooklyn.properties permissions
Repository: incubator-brooklyn
Updated Branches:
refs/heads/master 79530b3c1 -> 14af1a510
BROOKLYN-15: enforce brooklyn.properties permissions
- brooklyn.properties must be something like 600; otherwise
brooklyn will fail to start
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/266a2ef4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/266a2ef4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/266a2ef4
Branch: refs/heads/master
Commit: 266a2ef436a947f9b146e6715e0c4d815146e8bb
Parents: 79530b3
Author: Aled Sage <al...@gmail.com>
Authored: Fri Aug 8 18:25:46 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Aug 14 11:25:20 2014 +0100
----------------------------------------------------------------------
.../brooklyn/launcher/BrooklynLauncher.java | 50 +++++++++++++++--
.../brooklyn/launcher/BrooklynLauncherTest.java | 56 ++++++++++++++++++++
2 files changed, 103 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/266a2ef4/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index b8669c4..d3faeea 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -27,6 +27,7 @@ import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
import java.io.Closeable;
import java.io.File;
+import java.io.FileNotFoundException;
import java.io.StringReader;
import java.net.InetAddress;
import java.net.URI;
@@ -80,7 +81,10 @@ import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.exceptions.FatalConfigurationRuntimeException;
import brooklyn.util.exceptions.FatalRuntimeException;
import brooklyn.util.exceptions.RuntimeInterruptedException;
+import brooklyn.util.guava.Maybe;
+import brooklyn.util.io.FileUtil;
import brooklyn.util.net.Networking;
+import brooklyn.util.os.Os;
import brooklyn.util.stream.Streams;
import brooklyn.util.text.Strings;
import brooklyn.util.time.Duration;
@@ -150,7 +154,7 @@ public class BrooklynLauncher {
private CampPlatform campPlatform;
private boolean started;
- private String globalBrooklynPropertiesFile;
+ private String globalBrooklynPropertiesFile = Os.mergePaths(Os.home(), ".brooklyn", "brooklyn.properties");
private String localBrooklynPropertiesFile;
public List<Application> getApplications() {
@@ -440,8 +444,17 @@ public class BrooklynLauncher {
if (managementContext == null) {
if (brooklynProperties == null) {
Builder builder = new BrooklynProperties.Factory.Builder();
- if (globalBrooklynPropertiesFile != null) builder.globalPropertiesFile(globalBrooklynPropertiesFile);
- if (localBrooklynPropertiesFile != null) builder.localPropertiesFile(localBrooklynPropertiesFile);
+ if (globalBrooklynPropertiesFile != null) {
+ // brooklyn.properties stores passwords (web-console and cloud credentials),
+ // so ensure it has sensible permissions
+ checkFilePermissionsX00(globalBrooklynPropertiesFile);
+ builder.globalPropertiesFile(globalBrooklynPropertiesFile);
+ }
+ if (localBrooklynPropertiesFile != null) {
+ checkFileReadable(localBrooklynPropertiesFile);
+ checkFilePermissionsX00(localBrooklynPropertiesFile);
+ builder.localPropertiesFile(localBrooklynPropertiesFile);
+ }
managementContext = new LocalManagementContext(builder, brooklynAdditionalProperties);
} else {
managementContext = new LocalManagementContext(brooklynProperties, brooklynAdditionalProperties);
@@ -485,6 +498,37 @@ public class BrooklynLauncher {
return this;
}
+ private void checkFileReadable(String file) {
+ File f = new File(Os.tidyPath(file));
+ if (!f.exists()) {
+ throw new FatalRuntimeException("File "+file+" does not exist");
+ }
+ if (!f.isFile()) {
+ throw new FatalRuntimeException(file+" is not a file");
+ }
+ if (!f.canRead()) {
+ throw new FatalRuntimeException(file+" is not readable");
+ }
+ }
+
+ private void checkFilePermissionsX00(String file) {
+ File f = new File(Os.tidyPath(file));
+ if (f.exists() && f.isFile() && f.canRead()) {
+ try {
+ Maybe<String> permission = FileUtil.getFilePermissions(f);
+ if (permission.isAbsent()) {
+ LOG.debug("Could not determine permissions of global brooklyn properties file; assuming ok: "+f);
+ } else {
+ if (!permission.get().substring(4).equals("------")) {
+ throw new FatalRuntimeException("Invalid permissions for file "+file+"; expected 600 but was "+permission.get());
+ }
+ }
+ } catch (FileNotFoundException e) {
+ throw Exceptions.propagate(e); // should not happen; did f.exists() above
+ }
+ }
+ }
+
private void handleSubsystemStartupError(boolean ignoreSuchErrors, String system, Exception e) {
Exceptions.propagateIfFatal(e);
if (ignoreSuchErrors) {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/266a2ef4/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherTest.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherTest.java b/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherTest.java
index d8a7843..1d59bd2 100644
--- a/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherTest.java
+++ b/usage/launcher/src/test/java/brooklyn/launcher/BrooklynLauncherTest.java
@@ -44,6 +44,8 @@ import brooklyn.test.HttpTestUtils;
import brooklyn.test.entity.TestApplication;
import brooklyn.test.entity.TestApplicationImpl;
import brooklyn.test.entity.TestEntity;
+import brooklyn.util.exceptions.FatalRuntimeException;
+import brooklyn.util.io.FileUtil;
import brooklyn.util.os.Os;
import brooklyn.util.text.Strings;
@@ -219,6 +221,7 @@ public class BrooklynLauncherTest {
@Test
public void testReloadBrooklynPropertiesFromFile() throws Exception {
File globalPropertiesFile = File.createTempFile("local-brooklyn-properties-test", ".properties");
+ FileUtil.setFilePermissionsTo600(globalPropertiesFile);
try {
String property = "mykey=myval";
Files.write(property, globalPropertiesFile, Charsets.UTF_8);
@@ -237,6 +240,59 @@ public class BrooklynLauncherTest {
}
}
+ @Test(groups="Integration")
+ public void testChecksGlobalBrooklynPropertiesPermissionsX00() throws Exception {
+ File propsFile = File.createTempFile("testChecksGlobalBrooklynPropertiesPermissionsX00", ".properties");
+ propsFile.setReadable(true, false);
+ try {
+ launcher = BrooklynLauncher.newInstance()
+ .webconsole(false)
+ .globalBrooklynPropertiesFile(propsFile.getAbsolutePath())
+ .start();
+
+ assertEquals(launcher.getServerDetails().getManagementContext().getConfig().getFirst("mykey"), "myval");
+ } catch (FatalRuntimeException e) {
+ if (!e.toString().contains("Invalid permissions for file")) throw e;
+ } finally {
+ propsFile.delete();
+ }
+ }
+
+ @Test(groups="Integration")
+ public void testChecksLocalBrooklynPropertiesPermissionsX00() throws Exception {
+ File propsFile = File.createTempFile("testChecksLocalBrooklynPropertiesPermissionsX00", ".properties");
+ propsFile.setReadable(true, false);
+ try {
+ launcher = BrooklynLauncher.newInstance()
+ .webconsole(false)
+ .localBrooklynPropertiesFile(propsFile.getAbsolutePath())
+ .start();
+
+ assertEquals(launcher.getServerDetails().getManagementContext().getConfig().getFirst("mykey"), "myval");
+ } catch (FatalRuntimeException e) {
+ if (!e.toString().contains("Invalid permissions for file")) throw e;
+ } finally {
+ propsFile.delete();
+ }
+ }
+
+ @Test(groups="Integration")
+ public void testStartsWithBrooklynPropertiesPermissionsX00() throws Exception {
+ File globalPropsFile = File.createTempFile("testChecksLocalBrooklynPropertiesPermissionsX00_global", ".properties");
+ File localPropsFile = File.createTempFile("testChecksLocalBrooklynPropertiesPermissionsX00_local", ".properties");
+ FileUtil.setFilePermissionsTo600(globalPropsFile);
+ FileUtil.setFilePermissionsTo600(localPropsFile);
+ try {
+ launcher = BrooklynLauncher.newInstance()
+ .webconsole(false)
+ .localBrooklynPropertiesFile(globalPropsFile.getAbsolutePath())
+ .start();
+ } finally {
+ globalPropsFile.delete();
+ localPropsFile.delete();
+ }
+ }
+
private void assertOnlyApp(BrooklynLauncher launcher, Class<? extends Application> expectedType) {
assertEquals(launcher.getApplications().size(), 1, "apps="+launcher.getApplications());
assertNotNull(Iterables.find(launcher.getApplications(), Predicates.instanceOf(TestApplication.class), null), "apps="+launcher.getApplications());
[3/3] git commit: This closes #115
Posted by al...@apache.org.
This closes #115
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/14af1a51
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/14af1a51
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/14af1a51
Branch: refs/heads/master
Commit: 14af1a510dd669b285749a43a12a9caac74ac75e
Parents: 79530b3 b10da7e
Author: Aled Sage <al...@gmail.com>
Authored: Thu Aug 14 11:50:35 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Aug 14 11:50:35 2014 +0100
----------------------------------------------------------------------
.../brooklyn/launcher/BrooklynLauncher.java | 45 ++++++++++++++--
.../brooklyn/launcher/BrooklynLauncherTest.java | 56 ++++++++++++++++++++
.../main/java/brooklyn/util/io/FileUtil.java | 23 +++++---
3 files changed, 114 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
[2/3] git commit: Incorporate review comments (PRs #115 and #114)
Posted by al...@apache.org.
Incorporate review comments (PRs #115 and #114)
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b10da7ef
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b10da7ef
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b10da7ef
Branch: refs/heads/master
Commit: b10da7effb1e6f9e874008993f5dfa0eadb38d31
Parents: 266a2ef
Author: Aled Sage <al...@gmail.com>
Authored: Thu Aug 14 11:47:09 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Aug 14 11:47:09 2014 +0100
----------------------------------------------------------------------
.../brooklyn/launcher/BrooklynLauncher.java | 21 +++++++-----------
.../main/java/brooklyn/util/io/FileUtil.java | 23 ++++++++++++++------
2 files changed, 24 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b10da7ef/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index d3faeea..083763d 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -27,7 +27,6 @@ import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
import java.io.Closeable;
import java.io.File;
-import java.io.FileNotFoundException;
import java.io.StringReader;
import java.net.InetAddress;
import java.net.URI;
@@ -447,6 +446,7 @@ public class BrooklynLauncher {
if (globalBrooklynPropertiesFile != null) {
// brooklyn.properties stores passwords (web-console and cloud credentials),
// so ensure it has sensible permissions
+ checkFileReadable(globalBrooklynPropertiesFile);
checkFilePermissionsX00(globalBrooklynPropertiesFile);
builder.globalPropertiesFile(globalBrooklynPropertiesFile);
}
@@ -513,18 +513,13 @@ public class BrooklynLauncher {
private void checkFilePermissionsX00(String file) {
File f = new File(Os.tidyPath(file));
- if (f.exists() && f.isFile() && f.canRead()) {
- try {
- Maybe<String> permission = FileUtil.getFilePermissions(f);
- if (permission.isAbsent()) {
- LOG.debug("Could not determine permissions of global brooklyn properties file; assuming ok: "+f);
- } else {
- if (!permission.get().substring(4).equals("------")) {
- throw new FatalRuntimeException("Invalid permissions for file "+file+"; expected 600 but was "+permission.get());
- }
- }
- } catch (FileNotFoundException e) {
- throw Exceptions.propagate(e); // should not happen; did f.exists() above
+
+ Maybe<String> permission = FileUtil.getFilePermissions(f);
+ if (permission.isAbsent()) {
+ LOG.debug("Could not determine permissions of file; assuming ok: "+f);
+ } else {
+ if (!permission.get().substring(4).equals("------")) {
+ throw new FatalRuntimeException("Invalid permissions for file "+file+"; expected ?00 but was "+permission.get());
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b10da7ef/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
index 208717a..bf8c2ab 100644
--- a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
+++ b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
@@ -20,7 +20,6 @@ package brooklyn.util.io;
import java.io.ByteArrayOutputStream;
import java.io.File;
-import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -59,8 +58,13 @@ public class FileUtil {
if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}",
new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
} else {
- LOG.warn("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
- new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ if (Os.isMicrosoftWindows()) {
+ if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 700 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+ new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ } else {
+ LOG.warn("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+ new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ }
loggedSetFilePermissionsWarning = true;
}
}
@@ -86,8 +90,13 @@ public class FileUtil {
if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}",
new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
} else {
- LOG.warn("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
- new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ if (Os.isMicrosoftWindows()) {
+ if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 600 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+ new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ } else {
+ LOG.warn("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+ new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+ }
loggedSetFilePermissionsWarning = true;
}
}
@@ -121,8 +130,8 @@ public class FileUtil {
* @return The file permission (in a form like "-rwxr--r--"), or null if the permissions could not be determined.
*/
@Beta
- public static Maybe<String> getFilePermissions(File file) throws FileNotFoundException {
- if (!file.exists()) throw new FileNotFoundException();
+ public static Maybe<String> getFilePermissions(File file) {
+ if (!file.exists()) return Maybe.absent("File "+file+" does not exist");
if (Os.isMicrosoftWindows()) {
return Maybe.absent("Cannot determine permissions on windows");