You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by gr...@apache.org on 2016/12/01 16:03:25 UTC
[2/4] brooklyn-server git commit: BROOKLYN-405: ssh doesn't log
password environment variables
BROOKLYN-405: ssh doesn't log password environment variables
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/171843ec
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/171843ec
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/171843ec
Branch: refs/heads/master
Commit: 171843ecef4b5432c91840de15399bf7aa42fed9
Parents: e5b40cc
Author: Aled Sage <al...@gmail.com>
Authored: Thu Dec 1 08:50:22 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Dec 1 15:38:53 2016 +0000
----------------------------------------------------------------------
.../location/ssh/SshMachineLocation.java | 1 -
.../system/internal/ExecWithLoggingHelpers.java | 3 +-
.../ssh/SshMachineLocationIntegrationTest.java | 7 ++++
.../location/ssh/SshMachineLocationTest.java | 34 ++++++++++++++++++
.../org/apache/brooklyn/test/LogWatcher.java | 36 ++++++++++++++------
5 files changed, 69 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/171843ec/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
index 40ce7a6..b611c3c 100644
--- a/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
+++ b/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java
@@ -30,7 +30,6 @@ import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.io.Reader;
-import java.io.StringReader;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Socket;
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/171843ec/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/ExecWithLoggingHelpers.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/ExecWithLoggingHelpers.java b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/ExecWithLoggingHelpers.java
index e96dce9..790dc18 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/ExecWithLoggingHelpers.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/system/internal/ExecWithLoggingHelpers.java
@@ -27,6 +27,7 @@ import java.util.Map;
import org.slf4j.Logger;
import org.apache.brooklyn.config.ConfigKey;
+import org.apache.brooklyn.core.config.Sanitizer;
import org.apache.brooklyn.location.ssh.SshMachineLocation;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.config.ConfigBag;
@@ -112,7 +113,7 @@ public abstract class ExecWithLoggingHelpers {
String allcmds = (Strings.isBlank(expectedCommandHeaders) ? "" : expectedCommandHeaders + " ; ") + Strings.join(commands, " ; ");
commandLogger.debug("{}, initiating "+shortName.toLowerCase()+" on machine {}{}: {}",
new Object[] {summaryForLogging, getTargetName(),
- env!=null && !env.isEmpty() ? " (env "+env+")": "", allcmds});
+ env!=null && !env.isEmpty() ? " (env "+Sanitizer.sanitize(env)+")": "", allcmds});
}
if (commands.isEmpty()) {
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/171843ec/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
index 1def66f..081d0a8 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationIntegrationTest.java
@@ -103,6 +103,13 @@ public class SshMachineLocationIntegrationTest extends SshMachineLocationTest {
super.testIsSshableWhenTrue();
}
+ // Overridden just to make it integration (because `newHost()` returns a real ssh'ing host)
+ @Test(groups="Integration")
+ @Override
+ public void testDoesNotLogPasswordsInEnvironmentVariables() {
+ super.testDoesNotLogPasswordsInEnvironmentVariables();
+ }
+
// Overrides super, because expect real machine details (rather than asserting our stub data)
@Test(groups = "Integration")
@Override
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/171843ec/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
index c09d790..3b6239c 100644
--- a/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
+++ b/core/src/test/java/org/apache/brooklyn/location/ssh/SshMachineLocationTest.java
@@ -40,6 +40,7 @@ import org.apache.brooklyn.api.location.LocationSpec;
import org.apache.brooklyn.api.location.MachineDetails;
import org.apache.brooklyn.api.location.MachineLocation;
import org.apache.brooklyn.api.location.PortRange;
+import org.apache.brooklyn.core.BrooklynLogging;
import org.apache.brooklyn.core.effector.EffectorBody;
import org.apache.brooklyn.core.effector.EffectorTaskTest;
import org.apache.brooklyn.core.effector.Effectors;
@@ -53,10 +54,13 @@ import org.apache.brooklyn.core.location.Machines;
import org.apache.brooklyn.core.location.PortRanges;
import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
import org.apache.brooklyn.core.test.entity.TestApplication;
+import org.apache.brooklyn.test.LogWatcher;
+import org.apache.brooklyn.test.LogWatcher.EventPredicates;
import org.apache.brooklyn.util.collections.MutableMap;
import org.apache.brooklyn.util.core.config.ConfigBag;
import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool;
import org.apache.brooklyn.util.core.internal.ssh.RecordingSshTool.CustomResponse;
+import org.apache.brooklyn.util.core.internal.ssh.sshj.SshjTool;
import org.apache.brooklyn.util.core.task.BasicExecutionContext;
import org.apache.brooklyn.util.core.task.BasicExecutionManager;
import org.apache.brooklyn.util.guava.Maybe;
@@ -70,9 +74,15 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import com.google.common.base.Joiner;
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
/**
* Test the {@link SshMachineLocation} implementation of the {@link Location} interface.
@@ -273,4 +283,28 @@ public class SshMachineLocationTest extends BrooklynAppUnitTestSupport {
assertEquals(host.obtainPort(PortRanges.fromString("8000")), -1);
assertEquals(host.obtainPort(PortRanges.fromString("8000+")), 8001);
}
+
+ @Test
+ public void testDoesNotLogPasswordsInEnvironmentVariables() {
+ List<String> loggerNames = ImmutableList.of(
+ SshMachineLocation.class.getName(),
+ BrooklynLogging.SSH_IO,
+ SshjTool.class.getName());
+ ch.qos.logback.classic.Level logLevel = ch.qos.logback.classic.Level.DEBUG;
+ Predicate<ILoggingEvent> filter = Predicates.or(
+ EventPredicates.containsMessage("DB_PASSWORD"),
+ EventPredicates.containsMessage("mypassword"));
+ LogWatcher watcher = new LogWatcher(loggerNames, logLevel, filter);
+
+ watcher.start();
+ try {
+ host.execCommands("mySummary", ImmutableList.of("true"), ImmutableMap.of("DB_PASSWORD", "mypassword"));
+ watcher.assertHasEventEventually();
+
+ Optional<ILoggingEvent> eventWithPasswd = Iterables.tryFind(watcher.getEvents(), EventPredicates.containsMessage("mypassword"));
+ assertFalse(eventWithPasswd.isPresent(), "event="+eventWithPasswd);
+ } finally {
+ watcher.close();
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/171843ec/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
----------------------------------------------------------------------
diff --git a/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java b/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
index 6baa852..6495db3 100644
--- a/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
+++ b/test-support/src/main/java/org/apache/brooklyn/test/LogWatcher.java
@@ -25,6 +25,7 @@ import static org.testng.Assert.assertFalse;
import java.io.Closeable;
import java.util.Collections;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import org.mockito.Mockito;
@@ -37,6 +38,7 @@ import com.google.common.annotations.Beta;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.spi.ILoggingEvent;
@@ -88,13 +90,20 @@ public class LogWatcher implements Closeable {
private final List<ILoggingEvent> events = Collections.synchronizedList(Lists.<ILoggingEvent>newLinkedList());
private final AtomicBoolean closed = new AtomicBoolean();
private final ch.qos.logback.classic.Level loggerLevel;
- private final ch.qos.logback.classic.Logger watchedLogger;
private final Appender<ILoggingEvent> appender;
- private volatile Level origLevel;
+ private final List<ch.qos.logback.classic.Logger> watchedLoggers = Lists.newArrayList();
+ private volatile Map<ch.qos.logback.classic.Logger, Level> origLevels = Maps.newLinkedHashMap();
+
+ public LogWatcher(String loggerName, ch.qos.logback.classic.Level loggerLevel, final Predicate<? super ILoggingEvent> filter) {
+ this(ImmutableList.of(checkNotNull(loggerName, "loggerName")), loggerLevel, filter);
+ }
@SuppressWarnings("unchecked")
- public LogWatcher(String loggerName, ch.qos.logback.classic.Level loggerLevel, final Predicate<? super ILoggingEvent> filter) {
- this.watchedLogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(checkNotNull(loggerName, "loggerName"));
+ public LogWatcher(Iterable<String> loggerNames, ch.qos.logback.classic.Level loggerLevel, final Predicate<? super ILoggingEvent> filter) {
+ for (String loggerName : loggerNames) {
+ Logger logger = LoggerFactory.getLogger(checkNotNull(loggerName, "loggerName"));
+ watchedLoggers.add((ch.qos.logback.classic.Logger) logger);
+ }
this.loggerLevel = checkNotNull(loggerLevel, "loggerLevel");
this.appender = Mockito.mock(Appender.class);
@@ -115,18 +124,25 @@ public class LogWatcher implements Closeable {
public void start() {
checkState(!closed.get(), "Cannot start LogWatcher after closed");
- origLevel = watchedLogger.getLevel();
- watchedLogger.setLevel(loggerLevel);
- watchedLogger.addAppender(appender);
+ for (ch.qos.logback.classic.Logger watchedLogger : watchedLoggers) {
+ origLevels.put(watchedLogger, watchedLogger.getLevel());
+ watchedLogger.setLevel(loggerLevel);
+ watchedLogger.addAppender(appender);
+ }
}
@Override
public void close() {
if (closed.compareAndSet(false, true)) {
- if (watchedLogger != null) {
- if (origLevel != null) watchedLogger.setLevel(origLevel);
- watchedLogger.detachAppender(appender);
+ if (watchedLoggers != null) {
+ for (ch.qos.logback.classic.Logger watchedLogger : watchedLoggers) {
+ Level origLevel = origLevels.get(watchedLogger);
+ if (origLevel != null) watchedLogger.setLevel(origLevel);
+ watchedLogger.detachAppender(appender);
+ }
}
+ watchedLoggers.clear();
+ origLevels.clear();
}
}