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 085865b7abd64..fcb9fd8bb8bb1 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.TransportVersion; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionRequest; @@ -82,6 +85,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; @@ -1656,26 +1660,6 @@ public boolean test(String profile, InetSocketAddress peerAddress) { } 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, @@ -1687,12 +1671,81 @@ public boolean test(String profile, InetSocketAddress peerAddress) { tracer, new TLSConfig(sslConfiguration, sslService::createSSLEngine), acceptPredicate, - 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); + } ); }); return httpTransports; } + // "public" so it can be used in tests + public static Netty4HttpServerTransport getHttpServerTransportWithHeadersValidator( + Settings settings, + NetworkService networkService, + ThreadPool threadPool, + NamedXContentRegistry xContentRegistry, + HttpServerTransport.Dispatcher dispatcher, + ClusterSettings clusterSettings, + SharedGroupFactory sharedGroupFactory, + Tracer tracer, + TLSConfig tlsConfig, + @Nullable AcceptChannelHandler.AcceptPredicate acceptPredicate, + HttpValidator httpValidator, + HttpValidator httpOptionsValidator + ) { + return getHttpServerTransportWithHeadersValidator( + settings, + networkService, + threadPool, + xContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory, + tracer, + tlsConfig, + acceptPredicate, + (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 Netty4HttpServerTransport 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 9c23b39a4dc6e..000a8a90b14bc 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 @@ -43,6 +43,7 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.threadpool.TestThreadPool; import org.elasticsearch.threadpool.ThreadPool; @@ -681,4 +682,170 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th testThreadPool.shutdownNow(); } } + + public void testOptionsRequestsFailWith400AndNoAuthn() throws Exception { + final Settings settings = Settings.builder().put(env.settings()).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(List.of()), + testThreadPool, + xContentRegistry(), + dispatcher, + randomClusterSettings(), + new SharedGroupFactory(settings), + Tracer.NOOP, + TLSConfig.noTLS(), + null, + (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(); + } + } + }