diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index fc6e270670b67..f8fdcf7d131ba 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -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; @@ -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; @@ -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, @@ -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, () -> { @@ -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, diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java index 9471ccd5a069a..8a2391e9d3c09 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4HttpServerTransportTests.java @@ -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; @@ -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; @@ -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; @@ -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; @@ -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 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(); + } + } + }