mirror of https://github.com/apache/jclouds.git
Fix bug where password information is printed in logs in case of exceptions
This commit is contained in:
parent
f2bc5ee6e7
commit
8f705fa4a4
|
@ -16,14 +16,11 @@
|
|||
*/
|
||||
package org.jclouds.openstack.keystone.v2_0.binders;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
|
||||
import java.util.Map;
|
||||
|
||||
import javax.inject.Inject;
|
||||
import javax.inject.Singleton;
|
||||
|
||||
import com.google.common.base.Predicates;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.ImmutableMap.Builder;
|
||||
import com.google.common.collect.Iterables;
|
||||
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 com.google.common.base.Strings;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.ImmutableMap.Builder;
|
||||
import com.google.common.collect.Iterables;
|
||||
import javax.inject.Inject;
|
||||
import javax.inject.Singleton;
|
||||
import java.util.Map;
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -16,10 +16,8 @@
|
|||
*/
|
||||
package org.jclouds.openstack.keystone.v2_0.handlers;
|
||||
|
||||
import static org.jclouds.http.HttpUtils.closeClientButKeepContentStream;
|
||||
|
||||
import javax.inject.Singleton;
|
||||
|
||||
import com.google.inject.Inject;
|
||||
import org.jclouds.Constants;
|
||||
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()) {
|
||||
|
|
|
@ -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.
|
||||
*/
|
||||
|
|
|
@ -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
|
||||
&& ("application/x-www-form-urlencoded".equals(request.getPayload().getContentMetadata().getContentType()) || request
|
||||
.getPayload() instanceof StringPayload)
|
||||
&& request.getPayload().getContentMetadata().getContentLength() != null
|
||||
&& request.getPayload().getContentMetadata().getContentLength() < 1024) {
|
||||
return requestPayloadIfStringOrFormIfNotReturnEmptyString(request, false);
|
||||
}
|
||||
|
||||
static String requestPayloadIfStringOrFormIfNotReturnEmptyString(HttpRequest request, boolean logSensitiveInformation) {
|
||||
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()
|
||||
.getRawContent() : Strings2.toStringAndClose(request.getPayload().openStream()));
|
||||
String logStatement;
|
||||
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) {
|
||||
}
|
||||
}
|
||||
|
|
|
@ -16,21 +16,32 @@
|
|||
*/
|
||||
package org.jclouds.http.internal;
|
||||
|
||||
import javax.annotation.Resource;
|
||||
import javax.inject.Named;
|
||||
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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();
|
||||
|
||||
}
|
||||
|
|
|
@ -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 org.jclouds.io.MutableContentMetadata;
|
||||
import org.jclouds.io.Payload;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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 org.jclouds.io.MutableContentMetadata;
|
||||
import org.jclouds.io.Payload;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -16,10 +16,14 @@
|
|||
*/
|
||||
package org.jclouds.logging.internal;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static org.jclouds.io.Payloads.newPayload;
|
||||
import static org.jclouds.util.Closeables2.closeQuietly;
|
||||
import com.google.common.io.ByteStreams;
|
||||
import com.google.common.io.FileBackedOutputStream;
|
||||
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 org.jclouds.io.MutableContentMetadata;
|
||||
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;
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static org.jclouds.io.Payloads.newPayload;
|
||||
import static org.jclouds.util.Closeables2.closeQuietly;
|
||||
|
||||
/**
|
||||
* 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 {
|
||||
wiredPayload = newPayload(output(oldContent.getRawContent()));
|
||||
} catch (UnsupportedOperationException e) {
|
||||
wiredPayload = newPayload(output(oldContent.getInput()));
|
||||
if (!oldContent.isSensitive() || isLogSensitiveInformation()) {
|
||||
try {
|
||||
wiredPayload = newPayload(output(oldContent.getRawContent()));
|
||||
} 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);
|
||||
}
|
||||
|
|
|
@ -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 org.jclouds.util.Strings2;
|
||||
import org.testng.annotations.Test;
|
||||
import static org.testng.Assert.assertEquals;
|
||||
import static org.testng.Assert.assertNotEquals;
|
||||
|
||||
@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");
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue