honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Just log 401 stacktraces (#55774) 

Ensure stacktraces of 401 errors for unauthenticated users are logged
but not returned in the response body.
This commit is contained in:
Albert Zaharovits 2020-06-10 20:39:32 +03:00 committed by GitHub
parent 44c3bb29e2
commit c57ccd99f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/src/main/java/org/elasticsearch/ElasticsearchException.java
View File

@ -88,7 +88,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
private static final String REASON = "reason";
private static final String CAUSED_BY = "caused_by";
private static final ParseField SUPPRESSED = new ParseField("suppressed");
private static final String STACK_TRACE = "stack_trace";
public static final String STACK_TRACE = "stack_trace";
private static final String HEADER = "header";
private static final String ERROR = "error";
private static final String ROOT_CAUSE = "root_cause";

48
server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java
View File

@ -19,8 +19,8 @@
package org.elasticsearch.rest;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.ElasticsearchException;
@ -46,6 +46,8 @@ public class BytesRestResponse extends RestResponse {
private static final String STATUS = "status";
private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed");
private final RestStatus status;
private final BytesReference content;
private final String contentType;
@ -92,8 +94,20 @@ public class BytesRestResponse extends RestResponse {
}
public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) throws IOException {
ToXContent.Params params = paramsFromRequest(channel.request());
if (params.paramAsBoolean(REST_EXCEPTION_SKIP_STACK_TRACE, REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) && e != null) {
// log exception only if it is not returned in the response
Supplier<?> messageSupplier = () -> new ParameterizedMessage("path: {}, params: {}",
channel.request().rawPath(), channel.request().params());
if (status.getStatus() < 500) {
SUPPRESSED_ERROR_LOGGER.debug(messageSupplier, e);
} else {
SUPPRESSED_ERROR_LOGGER.warn(messageSupplier, e);
}
}
this.status = status;
try (XContentBuilder builder = build(channel, status, e)) {
try (XContentBuilder builder = channel.newErrorBuilder()) {
build(builder, params, status, channel.detailedErrorsEnabled(), e);
this.content = BytesReference.bytes(builder);
this.contentType = builder.contentType().mediaType();
}
@ -117,28 +131,24 @@ public class BytesRestResponse extends RestResponse {
return this.status;
}
private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed");
private static XContentBuilder build(RestChannel channel, RestStatus status, Exception e) throws IOException {
ToXContent.Params params = channel.request();
if (params.paramAsBoolean("error_trace", !REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT)) {
private ToXContent.Params paramsFromRequest(RestRequest restRequest) {
ToXContent.Params params = restRequest;
if (params.paramAsBoolean("error_trace", !REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) && false == skipStackTrace()) {
params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params);
} else if (e != null) {
Supplier<?> messageSupplier = () -> new ParameterizedMessage("path: {}, params: {}",
channel.request().rawPath(), channel.request().params());
if (status.getStatus() < 500) {
SUPPRESSED_ERROR_LOGGER.debug(messageSupplier, e);
} else {
SUPPRESSED_ERROR_LOGGER.warn(messageSupplier, e);
}
}
return params;
}
XContentBuilder builder = channel.newErrorBuilder().startObject();
ElasticsearchException.generateFailureXContent(builder, params, e, channel.detailedErrorsEnabled());
protected boolean skipStackTrace() {
return false;
}
private void build(XContentBuilder builder, ToXContent.Params params, RestStatus status,
boolean detailedErrorsEnabled, Exception e) throws IOException {
builder.startObject();
ElasticsearchException.generateFailureXContent(builder, params, e, detailedErrorsEnabled);
builder.field(STATUS, status.getStatus());
builder.endObject();
return builder;
}
static BytesRestResponse createSimpleErrorResponse(RestChannel channel, RestStatus status, String errorMessage) throws IOException {

10
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java
View File

@ -9,6 +9,7 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.util.concurrent.ThreadContext;
@ -19,6 +20,7 @@ import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestRequest.Method;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.core.security.rest.RestRequestFilter;
import org.elasticsearch.xpack.security.authc.AuthenticationService;
import org.elasticsearch.xpack.security.authc.support.SecondaryAuthenticator;
@ -82,8 +84,14 @@ public class SecurityRestFilter implements RestHandler {
private void handleException(String actionType, RestRequest request, RestChannel channel, Exception e) {
logger.debug(new ParameterizedMessage("{} failed for REST request [{}]", actionType, request.uri()), e);
final RestStatus restStatus = ExceptionsHelper.status(e);
try {
channel.sendResponse(new BytesRestResponse(channel, e));
channel.sendResponse(new BytesRestResponse(channel, restStatus, e) {
@Override
protected boolean skipStackTrace() { return restStatus == RestStatus.UNAUTHORIZED; }
});
} catch (Exception inner) {
inner.addSuppressed(e);
logger.error((Supplier<?>) () ->

1
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java
View File

@ -164,7 +164,6 @@ public class SecurityTests extends ESTestCase {
assertEquals("Realm type [" + FileRealmSettings.TYPE + "] is already registered", e.getMessage());
}
public void testAuditEnabled() throws Exception {
Settings settings = Settings.builder().put(XPackSettings.AUDIT_ENABLED.getKey(), true).build();
Collection<Object> components = createComponents(settings);

49
x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java
View File

@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.rest;
import com.nimbusds.jose.util.StandardCharset;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.bytes.BytesArray;
@ -22,6 +23,7 @@ import org.elasticsearch.rest.BytesRestResponse;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.SecuritySettingsSourceField;
@ -39,11 +41,15 @@ import org.mockito.ArgumentCaptor;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Matchers.any;
@ -141,21 +147,54 @@ public class SecurityRestFilterTests extends ESTestCase {
verifyZeroInteractions(channel, authcService);
}
public void testProcessAuthenticationError() throws Exception {
RestRequest request = mock(RestRequest.class);
Exception exception = authenticationError("failed authc");
public void testProcessAuthenticationFailedNoTrace() throws Exception {
filter = new SecurityRestFilter(licenseState, threadContext, authcService, secondaryAuthenticator, restHandler, false);
testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " +
"cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, true, true, false);
testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " +
"cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, true, false, false);
testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " +
"cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, false, true, false);
testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " +
"cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, false, false, false);
testProcessAuthenticationFailed(new ElasticsearchException("dummy"), RestStatus.INTERNAL_SERVER_ERROR, false, false, false);
testProcessAuthenticationFailed(new IllegalArgumentException("dummy"), RestStatus.BAD_REQUEST, true, false, false);
testProcessAuthenticationFailed(new ElasticsearchException("dummy"), RestStatus.INTERNAL_SERVER_ERROR, false, true, false);
testProcessAuthenticationFailed(new IllegalArgumentException("dummy"), RestStatus.BAD_REQUEST, true, true, true);
}
private void testProcessAuthenticationFailed(Exception authnException, RestStatus expectedRestStatus, boolean errorTrace,
boolean detailedErrorsEnabled, boolean traceExists) throws Exception {
RestRequest request;
if (errorTrace != !ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT || randomBoolean()) {
request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
.withParams(Collections.unmodifiableMap(new HashMap<String, String>() {{
put("error_trace", Boolean.toString(errorTrace));
}})).build();
} else {
// sometimes do not fill in the default value
request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build();
}
doAnswer((i) -> {
ActionListener callback =
(ActionListener) i.getArguments()[1];
callback.onFailure(exception);
callback.onFailure(authnException);
return Void.TYPE;
}).when(authcService).authenticate(eq(request), any(ActionListener.class));
RestChannel channel = mock(RestChannel.class);
when(channel.detailedErrorsEnabled()).thenReturn(detailedErrorsEnabled);
when(channel.request()).thenReturn(request);
when(channel.newErrorBuilder()).thenReturn(JsonXContent.contentBuilder());
filter.handleRequest(request, channel, null);
ArgumentCaptor<BytesRestResponse> response = ArgumentCaptor.forClass(BytesRestResponse.class);
verify(channel).sendResponse(response.capture());
assertEquals(RestStatus.UNAUTHORIZED, response.getValue().status());
RestResponse restResponse = response.getValue();
assertThat(restResponse.status(), is(expectedRestStatus));
if (traceExists) {
assertThat(restResponse.content().utf8ToString(), containsString(ElasticsearchException.STACK_TRACE));
} else {
assertThat(restResponse.content().utf8ToString(), not(containsString(ElasticsearchException.STACK_TRACE)));
}
verifyZeroInteractions(restHandler);
}