Skip to content

Commit

Permalink
Reject OPTIONS requests with a body (elastic#96357)
Browse files Browse the repository at this point in the history 
Instead of not authN and letting them through,
this PR rejects OPTIONS requests with a body (400).

Relates elastic#95112
  • Loading branch information
@albertzaharovits
albertzaharovits committed Jun 15, 2023
1 parent 250dcf8 commit f76d8a0
Show file tree
Hide file tree
Showing 2 changed files with 244 additions and 21 deletions.
95 changes: 74 additions & 21 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
View file
Expand Up @@ -7,11 +7,14 @@
package org.elasticsearch.xpack.security;

import io.netty.channel.Channel;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpUtil;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest;
Expand Down Expand Up @@ -77,6 +80,7 @@
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.rest.RestHeaderDefinition;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.internal.ShardSearchRequest;
import org.elasticsearch.threadpool.ExecutorBuilder;
Expand Down Expand Up @@ -1502,26 +1506,6 @@ && getSslService().isSSLClientAuthEnabled(getSslService().getHttpTransportSSLCon
}
final AuthenticationService authenticationService = this.authcService.get();
final ThreadContext threadContext = this.threadContext.get();
final HttpValidator httpValidator = (httpRequest, channel, listener) -> {
HttpPreRequest httpPreRequest = HttpHeadersAuthenticatorUtils.asHttpPreRequest(httpRequest);
// step 1: Populate the thread context with credentials and any other HTTP request header values (eg run-as) that the
// authentication process looks for while doing its duty.
perRequestThreadContext.accept(httpPreRequest, threadContext);
populateClientCertificate.accept(channel, threadContext);
RemoteHostHeader.process(channel, threadContext);
// step 2: Run authentication on the now properly prepared thread-context.
// This inspects and modifies the thread context.
if (httpPreRequest.method() != RestRequest.Method.OPTIONS) {
authenticationService.authenticate(
httpPreRequest,
ActionListener.wrap(ignored -> listener.onResponse(null), listener::onFailure)
);
} else {
// allow for unauthenticated OPTIONS request
// this includes CORS preflight, and regular OPTIONS that return permitted methods for a given path
listener.onResponse(null);
}
};
return getHttpServerTransportWithHeadersValidator(
settings,
networkService,
Expand All @@ -1533,7 +1517,30 @@ && getSslService().isSSLClientAuthEnabled(getSslService().getHttpTransportSSLCon
getSslService(),
getNettySharedGroupFactory(settings),
clusterSettings,
httpValidator
(httpRequest, channel, listener) -> {
HttpPreRequest httpPreRequest = HttpHeadersAuthenticatorUtils.asHttpPreRequest(httpRequest);
// step 1: Populate the thread context with credentials and any other HTTP request header values (eg run-as) that the
// authentication process looks for while doing its duty.
perRequestThreadContext.accept(httpPreRequest, threadContext);
populateClientCertificate.accept(channel, threadContext);
RemoteHostHeader.process(channel, threadContext);
// step 2: Run authentication on the now properly prepared thread-context.
// This inspects and modifies the thread context.
authenticationService.authenticate(
httpPreRequest,
ActionListener.wrap(ignored -> listener.onResponse(null), listener::onFailure)
);
},
(httpRequest, channel, listener) -> {
// allow unauthenticated OPTIONS request through
// this includes CORS preflight, and regular OPTIONS that return permitted methods for a given path
// But still populate the thread context with the usual request headers (as for any other request that is dispatched)
HttpPreRequest httpPreRequest = HttpHeadersAuthenticatorUtils.asHttpPreRequest(httpRequest);
perRequestThreadContext.accept(httpPreRequest, threadContext);
populateClientCertificate.accept(channel, threadContext);
RemoteHostHeader.process(channel, threadContext);
listener.onResponse(null);
}
);
});
httpTransports.put(SecurityField.NIO, () -> {
Expand Down Expand Up @@ -1569,6 +1576,52 @@ protected void populatePerRequestThreadContext(RestRequest restRequest, ThreadCo
return httpTransports;
}

// "public" so it can be used in tests
public static SecurityNetty4HttpServerTransport getHttpServerTransportWithHeadersValidator(
Settings settings,
NetworkService networkService,
BigArrays bigArrays,
ThreadPool threadPool,
NamedXContentRegistry xContentRegistry,
HttpServerTransport.Dispatcher dispatcher,
IPFilter ipFilter,
SSLService sslService,
SharedGroupFactory sharedGroupFactory,
ClusterSettings clusterSettings,
HttpValidator httpValidator,
HttpValidator httpOptionsValidator
) {
return getHttpServerTransportWithHeadersValidator(
settings,
networkService,
bigArrays,
threadPool,
xContentRegistry,
dispatcher,
ipFilter,
sslService,
sharedGroupFactory,
clusterSettings,
(httpRequest, channel, listener) -> {
if (httpRequest.method() == HttpMethod.OPTIONS) {
if (HttpUtil.getContentLength(httpRequest, -1L) > 1 || HttpUtil.isTransferEncodingChunked(httpRequest)) {
// OPTIONS requests with a body are not supported
listener.onFailure(
new ElasticsearchStatusException(
"OPTIONS requests with a payload body are not supported",
RestStatus.BAD_REQUEST
)
);
} else {
httpOptionsValidator.validate(httpRequest, channel, listener);
}
} else {
httpValidator.validate(httpRequest, channel, listener);
}
}
);
}

// "public" so it can be used in tests
public static SecurityNetty4HttpServerTransport getHttpServerTransportWithHeadersValidator(
Settings settings,
Expand Down
170 changes: 170 additions & 0 deletions ...elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java
View file
Expand Up @@ -6,16 +6,19 @@
*/
package org.elasticsearch.xpack.security.transport.netty4;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
import io.netty.channel.ChannelHandler;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultHttpRequest;
import io.netty.handler.codec.http.DefaultLastHttpContent;
import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.HttpMessage;
import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpResponseStatus;
import io.netty.handler.ssl.SslHandler;
import io.netty.util.AsciiString;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.network.NetworkService;
Expand All @@ -26,6 +29,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.http.AbstractHttpServerTransportTestCase;
import org.elasticsearch.http.HttpHeadersValidationException;
import org.elasticsearch.http.HttpRequest;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.http.NullDispatcher;
Expand All @@ -35,6 +39,7 @@
import org.elasticsearch.http.netty4.internal.HttpHeadersWithAuthenticationContext;
import org.elasticsearch.rest.RestChannel;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.rest.FakeRestRequest;
import org.elasticsearch.threadpool.TestThreadPool;
import org.elasticsearch.threadpool.ThreadPool;
Expand All @@ -60,6 +65,7 @@
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -436,4 +442,168 @@ public void testHttpHeaderAuthnFaultyHeaderValidator() throws Exception {
testThreadPool.shutdownNow();
}
}

public void testOptionsRequestsFailWith400AndNoAuthn() throws Exception {
final Settings settings = Settings.builder().put(env.settings()).put(HTTP_SSL_ENABLED.getKey(), false).build();
AtomicReference<Throwable> badRequestCauseReference = new AtomicReference<>();
final HttpServerTransport.Dispatcher dispatcher = new HttpServerTransport.Dispatcher() {
@Override
public void dispatchRequest(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) {
logger.error("--> Unexpected dispatched request [" + FakeRestRequest.requestToString(channel.request()) + "]");
throw new AssertionError("Unexpected dispatched request");
}

@Override
public void dispatchBadRequest(final RestChannel channel, final ThreadContext threadContext, final Throwable cause) {
badRequestCauseReference.set(cause);
}
};
final ThreadPool testThreadPool = new TestThreadPool(TEST_MOCK_TRANSPORT_THREAD_PREFIX);
try (
Netty4HttpServerTransport transport = Security.getHttpServerTransportWithHeadersValidator(
settings,
new NetworkService(Collections.emptyList()),
mock(BigArrays.class),
testThreadPool,
xContentRegistry(),
dispatcher,
mock(IPFilter.class),
sslService,
new SharedGroupFactory(settings),
randomClusterSettings(),
(httpPreRequest, channel, listener) -> { throw new AssertionError("should not be invoked for OPTIONS requests"); },
(httpPreRequest, channel, listener) -> {
throw new AssertionError("should not be invoked for OPTIONS requests with a body");
}
)
) {
final ChannelHandler handler = transport.configureServerChannelHandler();
final EmbeddedChannel ch = new EmbeddedChannel(handler);
// OPTIONS request with fixed length content written in one chunk
{
ByteBuf buf = ch.alloc().buffer();
ByteBufUtil.copy(AsciiString.of("OPTIONS /url/whatever/fixed-length-single-chunk HTTP/1.1"), buf);
buf.writeByte(HttpConstants.LF);
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Host: localhost"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Accept: */*"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Content-Encoding: gzip"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(
AsciiString.of("Content-Type: " + randomFrom("text/plain; charset=utf-8", "application/json; charset=utf-8")),
buf
);
buf.writeByte(HttpConstants.LF);
}
String content = randomAlphaOfLengthBetween(4, 1024);
// having a "Content-Length" request header is what makes it "fixed length"
ByteBufUtil.copy(AsciiString.of("Content-Length: " + content.length()), buf);
buf.writeByte(HttpConstants.LF);
// end of headers
buf.writeByte(HttpConstants.LF);
ByteBufUtil.copy(AsciiString.of(content), buf);
// write everything in one single chunk
testThreadPool.generic().submit(() -> {
ch.writeInbound(buf);
ch.flushInbound();
}).get();
ch.runPendingTasks();
Throwable badRequestCause = badRequestCauseReference.get();
assertThat(badRequestCause, instanceOf(HttpHeadersValidationException.class));
assertThat(badRequestCause.getCause(), instanceOf(ElasticsearchException.class));
assertThat(((ElasticsearchException) badRequestCause.getCause()).status(), is(RestStatus.BAD_REQUEST));
assertThat(
((ElasticsearchException) badRequestCause.getCause()).getDetailedMessage(),
containsString("OPTIONS requests with a payload body are not supported")
);
}
{
ByteBuf buf = ch.alloc().buffer();
ByteBufUtil.copy(AsciiString.of("OPTIONS /url/whatever/chunked-transfer?encoding HTTP/1.1"), buf);
buf.writeByte(HttpConstants.LF);
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Host: localhost"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Accept: */*"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Content-Encoding: gzip"), buf);
buf.writeByte(HttpConstants.LF);
}
if (randomBoolean()) {
ByteBufUtil.copy(
AsciiString.of("Content-Type: " + randomFrom("text/plain; charset=utf-8", "application/json; charset=utf-8")),
buf
);
buf.writeByte(HttpConstants.LF);
}
// do not write a "Content-Length" header to make the request "variable length"
if (randomBoolean()) {
ByteBufUtil.copy(AsciiString.of("Transfer-Encoding: " + randomFrom("chunked", "gzip, chunked")), buf);
} else {
ByteBufUtil.copy(AsciiString.of("Transfer-Encoding: chunked"), buf);
}
buf.writeByte(HttpConstants.LF);
buf.writeByte(HttpConstants.LF);
// maybe append some chunks as well
String[] contentParts = randomArray(0, 4, String[]::new, () -> randomAlphaOfLengthBetween(1, 64));
for (String content : contentParts) {
ByteBufUtil.copy(AsciiString.of(Integer.toHexString(content.length())), buf);
buf.writeByte(HttpConstants.CR);
buf.writeByte(HttpConstants.LF);
ByteBufUtil.copy(AsciiString.of(content), buf);
buf.writeByte(HttpConstants.CR);
buf.writeByte(HttpConstants.LF);
}
testThreadPool.generic().submit(() -> {
ch.writeInbound(buf);
ch.flushInbound();
}).get();
// append some more chunks as well
ByteBuf buf2 = ch.alloc().buffer();
contentParts = randomArray(1, 4, String[]::new, () -> randomAlphaOfLengthBetween(1, 64));
for (String content : contentParts) {
ByteBufUtil.copy(AsciiString.of(Integer.toHexString(content.length())), buf2);
buf2.writeByte(HttpConstants.CR);
buf2.writeByte(HttpConstants.LF);
ByteBufUtil.copy(AsciiString.of(content), buf2);
buf2.writeByte(HttpConstants.CR);
buf2.writeByte(HttpConstants.LF);
}
// finish chunked request
ByteBufUtil.copy(AsciiString.of("0"), buf2);
buf2.writeByte(HttpConstants.CR);
buf2.writeByte(HttpConstants.LF);
buf2.writeByte(HttpConstants.CR);
buf2.writeByte(HttpConstants.LF);
testThreadPool.generic().submit(() -> {
ch.writeInbound(buf2);
ch.flushInbound();
}).get();
ch.runPendingTasks();
Throwable badRequestCause = badRequestCauseReference.get();
assertThat(badRequestCause, instanceOf(HttpHeadersValidationException.class));
assertThat(badRequestCause.getCause(), instanceOf(ElasticsearchException.class));
assertThat(((ElasticsearchException) badRequestCause.getCause()).status(), is(RestStatus.BAD_REQUEST));
assertThat(
((ElasticsearchException) badRequestCause.getCause()).getDetailedMessage(),
containsString("OPTIONS requests with a payload body are not supported")
);
}
} finally {
testThreadPool.shutdownNow();
}
}

}

0 comments on commit f76d8a0

Please sign in to comment.