Fix bug where password information is printed in logs in case of exceptions

apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/binders/BindAuthToJsonPayload.java

@@ -16,14 +16,11 @@
  */
 package org.jclouds.openstack.keystone.v2_0.binders;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import com.google.common.base.Predicates;
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.Strings;
-
+import com.google.common.collect.ImmutableMap;
-import java.util.Map;
+import com.google.common.collect.ImmutableMap.Builder;
-
+import com.google.common.collect.Iterables;
-import javax.inject.Inject;
-import javax.inject.Singleton;
-
 import org.jclouds.http.HttpRequest;
 import org.jclouds.json.Json;
 import org.jclouds.openstack.keystone.v2_0.config.CredentialType;
@@ -31,11 +28,12 @@ import org.jclouds.rest.MapBinder;
 import org.jclouds.rest.binders.BindToJsonPayload;
 import org.jclouds.rest.internal.GeneratedHttpRequest;
 
-import com.google.common.base.Predicates;
+import javax.inject.Inject;
-import com.google.common.base.Strings;
+import javax.inject.Singleton;
-import com.google.common.collect.ImmutableMap;
+import java.util.Map;
-import com.google.common.collect.ImmutableMap.Builder;
+
-import com.google.common.collect.Iterables;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 
 @Singleton
 public class BindAuthToJsonPayload extends BindToJsonPayload implements MapBinder {
@@ -71,7 +69,9 @@ public class BindAuthToJsonPayload extends BindToJsonPayload implements MapBinde
          builder.put("tenantName", postParams.get("tenantName"));
       else if (!Strings.isNullOrEmpty((String) postParams.get("tenantId")))
           builder.put("tenantId", postParams.get("tenantId"));
-      return super.bindToRequest(request, ImmutableMap.of("auth", builder.build()));
+      R authRequest = super.bindToRequest(request, ImmutableMap.of("auth", builder.build()));
+      authRequest.getPayload().setSensitive(true);
+      return authRequest;
    }
 
 }

apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/handlers/KeystoneErrorHandler.java

@@ -16,10 +16,8 @@
  */
 package org.jclouds.openstack.keystone.v2_0.handlers;
 
-import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
+import com.google.inject.Inject;
-
+import org.jclouds.Constants;
-import javax.inject.Singleton;
-
 import org.jclouds.http.HttpCommand;
 import org.jclouds.http.HttpErrorHandler;
 import org.jclouds.http.HttpResponse;
@@ -27,19 +25,29 @@ import org.jclouds.http.HttpResponseException;
 import org.jclouds.rest.AuthorizationException;
 import org.jclouds.rest.ResourceNotFoundException;
 
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
+
 /**
  * This will parse and set an appropriate exception on the command object.
  */
 // TODO: is there error spec someplace? let's type errors, etc.
 @Singleton
 public class KeystoneErrorHandler implements HttpErrorHandler {
+
+   @Inject(optional = true)
+   @Named(Constants.PROPERTY_LOGGER_WIRE_LOG_SENSITIVE_INFO)
+   private boolean logSensitiveInformation = false;
+
    public void handleError(HttpCommand command, HttpResponse response) {
       // it is important to always read fully and close streams
       byte[] data = closeClientButKeepContentStream(response);
       String message = data != null ? new String(data) : null;
 
       Exception exception = message != null ? new HttpResponseException(command, response, message)
-               : new HttpResponseException(command, response);
+               : new HttpResponseException(command, response, logSensitiveInformation);
       message = message != null ? message : String.format("%s -> %s", command.getCurrentRequest().getRequestLine(),
                response.getStatusLine());
       switch (response.getStatusCode()) {

core/src/main/java/org/jclouds/Constants.java

@@ -191,6 +191,13 @@ public final class Constants {
     * trust self-signed certs
     */
    public static final String PROPERTY_TRUST_ALL_CERTS = "jclouds.trust-all-certs";
+   /**
+    * Boolean property.
+    * <p/>
+    * true to allow logging of sensitive information like passwords in the wire log
+    * default value is false
+    */
+   public static final String PROPERTY_LOGGER_WIRE_LOG_SENSITIVE_INFO = "jclouds.wire.log.sensitive";
    /**
     * Name of the logger that records all http headers from the client and the server.
     */

core/src/main/java/org/jclouds/http/HttpResponseException.java

@@ -16,12 +16,13 @@
  */
 package org.jclouds.http;
 
-import java.io.IOException;
+import org.jclouds.io.Payload;
-
 import org.jclouds.io.payloads.StringPayload;
 import org.jclouds.javax.annotation.Nullable;
 import org.jclouds.util.Strings2;
 
+import java.io.IOException;
+
 /**
  * Represents an error obtained from an HttpResponse.
  */
@@ -71,20 +72,34 @@ public class HttpResponseException extends RuntimeException {
    }
 
    public HttpResponseException(HttpCommand command, HttpResponse response) {
+      this(command, response, false);
+   }
+   public HttpResponseException(HttpCommand command, HttpResponse response, boolean logSensitiveInformation) {
       this(String.format("request: %s %sfailed with response: %s", command.getCurrentRequest().getRequestLine(),
-            requestPayloadIfStringOrFormIfNotReturnEmptyString(command.getCurrentRequest()),
+            requestPayloadIfStringOrFormIfNotReturnEmptyString(command.getCurrentRequest(), logSensitiveInformation),
             response.getStatusLine()), command, response);
    }
 
    static String requestPayloadIfStringOrFormIfNotReturnEmptyString(HttpRequest request) {
-      if (request.getPayload() != null
+      return requestPayloadIfStringOrFormIfNotReturnEmptyString(request, false);
-            && ("application/x-www-form-urlencoded".equals(request.getPayload().getContentMetadata().getContentType()) || request
+   }
-                  .getPayload() instanceof StringPayload)
+
-            && request.getPayload().getContentMetadata().getContentLength() != null
+   static String requestPayloadIfStringOrFormIfNotReturnEmptyString(HttpRequest request, boolean logSensitiveInformation) {
-            && request.getPayload().getContentMetadata().getContentLength() < 1024) {
+      Payload payload = request.getPayload();
+      if (payload != null
+            && ("application/x-www-form-urlencoded".equals(payload.getContentMetadata().getContentType()) || payload instanceof StringPayload)
+            && payload.getContentMetadata().getContentLength() != null
+            && payload.getContentMetadata().getContentLength() < 1024) {
          try {
-            return String.format(" [%s] ", request.getPayload() instanceof StringPayload ? request.getPayload()
+            String logStatement;
-                  .getRawContent() : Strings2.toStringAndClose(request.getPayload().openStream()));
+            if (payload.isSensitive() && !logSensitiveInformation) {
+               logStatement = "Sensitive data in payload, use PROPERTY_LOGGER_WIRE_LOG_SENSITIVE_INFO override to enable logging this data.";
+            } else if (payload instanceof StringPayload) {
+               logStatement = payload.getRawContent().toString();
+            } else {
+               logStatement = Strings2.toStringAndClose(payload.openStream());
+            }
+            return String.format(" [%s] ", logStatement);
          } catch (IOException e) {
          }
       }

core/src/main/java/org/jclouds/http/internal/HttpWire.java

@@ -16,21 +16,32 @@
  */
 package org.jclouds.http.internal;
 
-import javax.annotation.Resource;
+import com.google.common.annotations.VisibleForTesting;
-import javax.inject.Named;
+import com.google.inject.Inject;
-
 import org.jclouds.Constants;
 import org.jclouds.logging.Logger;
 import org.jclouds.logging.internal.Wire;
 
+import javax.annotation.Resource;
+import javax.inject.Named;
+
 public class HttpWire extends Wire {
 
    @Resource
    @Named(Constants.LOGGER_HTTP_WIRE)
    Logger wireLog = Logger.NULL;
 
+   @VisibleForTesting
+   @Inject(optional = true)
+   @Named(Constants.PROPERTY_LOGGER_WIRE_LOG_SENSITIVE_INFO)
+   boolean logSensitiveInformation = false;
+
    public Logger getWireLog() {
       return wireLog;
    }
 
+   @Override
+   protected boolean isLogSensitiveInformation() {
+      return logSensitiveInformation;
+   }
 }

core/src/main/java/org/jclouds/io/Payload.java

@@ -17,8 +17,8 @@
 package org.jclouds.io;
 
 import java.io.Closeable;
-import java.io.InputStream;
 import java.io.IOException;
+import java.io.InputStream;
 
 public interface Payload extends Closeable {
 
@@ -54,4 +54,16 @@ public interface Payload extends Closeable {
 
    void setContentMetadata(MutableContentMetadata in);
 
+   /**
+    * Sets whether the payload contains sensitive information. This is used when trying to decide whether to print out the
+    * payload information or not in logs
+    */
+   void setSensitive(boolean isSensitive);
+
+   /**
+    * Returns whether the payload contains sensitive information. This is used when trying to decide whether to print out the
+    * payload information or not in logs
+    */
+   boolean isSensitive();
+
 }

core/src/main/java/org/jclouds/io/payloads/BasePayload.java

@@ -16,20 +16,20 @@
  */
 package org.jclouds.io.payloads;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.Throwables;
+import org.jclouds.io.MutableContentMetadata;
+import org.jclouds.io.Payload;
 
 import java.io.IOException;
 import java.io.InputStream;
 
-import com.google.common.base.Throwables;
+import static com.google.common.base.Preconditions.checkNotNull;
-
-import org.jclouds.io.MutableContentMetadata;
-import org.jclouds.io.Payload;
 
 public abstract class BasePayload<V> implements Payload {
    protected final V content;
    protected transient volatile boolean written;
    protected MutableContentMetadata contentMetadata;
+   private boolean isSensitive;
 
    protected BasePayload(V content) {
       this(content, new BaseMutableContentMetadata());
@@ -84,7 +84,7 @@ public abstract class BasePayload<V> implements Payload {
 
    @Override
    public String toString() {
-      return "[content=" + (content != null) + ", contentMetadata=" + contentMetadata + ", written=" + written + "]";
+      return "[content=" + (content != null) + ", contentMetadata=" + contentMetadata + ", written=" + written + ", isSensitive=" + isSensitive + "]";
    }
 
    /**
@@ -128,4 +128,13 @@ public abstract class BasePayload<V> implements Payload {
       this.contentMetadata = in;
    }
 
+   @Override
+   public void setSensitive(boolean isSensitive) {
+      this.isSensitive = isSensitive;
+   }
+
+   @Override
+   public boolean isSensitive() {
+      return this.isSensitive;
+   }
 }

core/src/main/java/org/jclouds/io/payloads/DelegatingPayload.java

@@ -16,19 +16,19 @@
  */
 package org.jclouds.io.payloads;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.base.Throwables;
+import org.jclouds.io.MutableContentMetadata;
+import org.jclouds.io.Payload;
 
 import java.io.IOException;
 import java.io.InputStream;
 
-import com.google.common.base.Throwables;
+import static com.google.common.base.Preconditions.checkNotNull;
-
-import org.jclouds.io.MutableContentMetadata;
-import org.jclouds.io.Payload;
 
 public class DelegatingPayload implements Payload {
 
    private final Payload delegate;
+   private boolean isSensitive;
 
    public DelegatingPayload(Payload delegate) {
       this.delegate = checkNotNull(delegate, "delegate");
@@ -110,4 +110,14 @@ public class DelegatingPayload implements Payload {
    public void setContentMetadata(MutableContentMetadata in) {
        delegate.setContentMetadata(in);
    }
+
+   @Override
+   public void setSensitive(boolean isSensitive) {
+      this.isSensitive = isSensitive;
+   }
+
+   @Override
+   public boolean isSensitive() {
+      return this.isSensitive;
+   }
 }

core/src/main/java/org/jclouds/logging/internal/Wire.java

@@ -16,10 +16,14 @@
  */
 package org.jclouds.logging.internal;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.io.ByteStreams;
-import static org.jclouds.io.Payloads.newPayload;
+import com.google.common.io.FileBackedOutputStream;
-import static org.jclouds.util.Closeables2.closeQuietly;
+import org.jclouds.io.MutableContentMetadata;
+import org.jclouds.io.Payload;
+import org.jclouds.io.PayloadEnclosing;
+import org.jclouds.logging.Logger;
 
+import javax.annotation.Resource;
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
@@ -28,15 +32,9 @@ import java.io.FilterInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 
-import javax.annotation.Resource;
+import static com.google.common.base.Preconditions.checkNotNull;
-
+import static org.jclouds.io.Payloads.newPayload;
-import org.jclouds.io.MutableContentMetadata;
+import static org.jclouds.util.Closeables2.closeQuietly;
-import org.jclouds.io.Payload;
-import org.jclouds.io.PayloadEnclosing;
-import org.jclouds.logging.Logger;
-
-import com.google.common.io.ByteStreams;
-import com.google.common.io.FileBackedOutputStream;
 
 /**
  * Logs data to the wire LOG, similar to {@code org.apache.HttpWire.impl.conn.Wire}
@@ -48,6 +46,10 @@ public abstract class Wire {
 
    protected abstract Logger getWireLog();
 
+   protected boolean isLogSensitiveInformation() {
+      return false;
+   }
+
    private void wire(String header, InputStream instream) {
       StringBuilder buffer = new StringBuilder();
       int ch;
@@ -119,7 +121,13 @@ public abstract class Wire {
 
    public void input(PayloadEnclosing request) {
       Payload oldContent = request.getPayload();
-      Payload wiredPayload = newPayload(input(oldContent.getInput()));
+      Payload wiredPayload;
+      if (!oldContent.isSensitive() || isLogSensitiveInformation()) {
+         wiredPayload = newPayload(input(oldContent.getInput()));
+      } else {
+         wiredPayload = newPayload(oldContent.getInput());
+      }
+      wiredPayload.setSensitive(oldContent.isSensitive());
       copyPayloadMetadata(oldContent, wiredPayload);
       request.setPayload(wiredPayload);
    }
@@ -127,11 +135,21 @@ public abstract class Wire {
    public void output(PayloadEnclosing request) {
       Payload oldContent = request.getPayload();
       Payload wiredPayload;
-      try {
+      if (!oldContent.isSensitive() || isLogSensitiveInformation()) {
-         wiredPayload = newPayload(output(oldContent.getRawContent()));
+         try {
-      } catch (UnsupportedOperationException e) {
+            wiredPayload = newPayload(output(oldContent.getRawContent()));
-         wiredPayload = newPayload(output(oldContent.getInput()));
+         } catch (UnsupportedOperationException e) {
+            wiredPayload = newPayload(output(oldContent.getInput()));
+         }
+      } else {
+         try {
+            wiredPayload = newPayload(oldContent.getRawContent());
+         } catch (UnsupportedOperationException e) {
+            wiredPayload = newPayload(oldContent.getInput());
+         }
+         output("Sensitive data in payload, use PROPERTY_LOGGER_WIRE_LOG_SENSITIVE_INFO override to enable logging this data.");
       }
+      wiredPayload.setSensitive(oldContent.isSensitive());
       copyPayloadMetadata(oldContent, wiredPayload);
       request.setPayload(wiredPayload);
    }

core/src/test/java/org/jclouds/http/internal/WireTest.java

@@ -16,14 +16,18 @@
  */
 package org.jclouds.http.internal;
 
-import static org.testng.Assert.assertEquals;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.io.PayloadEnclosing;
+import org.jclouds.io.payloads.StringPayload;
+import org.jclouds.logging.Logger;
+import org.jclouds.util.Strings2;
+import org.testng.annotations.Test;
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 
-import org.jclouds.logging.Logger;
+import static org.testng.Assert.assertEquals;
-import org.jclouds.util.Strings2;
+import static org.testng.Assert.assertNotEquals;
-import org.testng.annotations.Test;
 
 @Test(groups = "unit", sequential = true)
 public class WireTest {
@@ -130,4 +134,48 @@ public class WireTest {
       wire.output("foo");
       assertEquals(((BufferLogger) wire.getWireLog()).buff.toString(), ">> \"foo\"");
    }
+
+   @Test
+   public void testInputPayload() throws Exception {
+      HttpWire wire = setUp();
+      StringPayload payload = new StringPayload("foo");
+      PayloadEnclosing request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.input(request);
+      BufferLogger wireLog = (BufferLogger) wire.getWireLog();
+      assertEquals(wireLog.buff.toString(), "<< \"foo\"", "Expected payload to be printed in logs");
+      wireLog.buff.setLength(0);
+
+      payload.setSensitive(true);
+      request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.input(request);
+      assertNotEquals(wireLog.buff.toString(), "<< \"foo\"", "Expected payload to NOT be printed in logs");
+      wireLog.buff.setLength(0);
+
+      wire.logSensitiveInformation = true;
+      request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.input(request);
+      assertEquals(wireLog.buff.toString(), "<< \"foo\"", "Expected payload to be printed in logs");
+   }
+
+   @Test
+   public void testOutputPayload() throws Exception {
+      HttpWire wire = setUp();
+      StringPayload payload = new StringPayload("foo");
+      PayloadEnclosing request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.output(request);
+      BufferLogger wireLog = (BufferLogger) wire.getWireLog();
+      assertEquals(wireLog.buff.toString(), ">> \"foo\"", "Expected payload to be printed in logs");
+      wireLog.buff.setLength(0);
+
+      payload.setSensitive(true);
+      request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.output(request);
+      assertNotEquals(wireLog.buff.toString(), ">> \"foo\"", "Expected payload to NOT be printed in logs");
+      wireLog.buff.setLength(0);
+
+      wire.logSensitiveInformation = true;
+      request = HttpRequest.builder().method("foo").endpoint("http://foo").payload(payload).build();
+      wire.output(request);
+      assertEquals(wireLog.buff.toString(), ">> \"foo\"", "Expected payload to be printed in logs");
+   }
 }