From 6b086dc7dbd7af9033cbe86b53a4b37c48a09588 Mon Sep 17 00:00:00 2001 From: jaymode Date: Fri, 26 Jun 2015 10:56:44 -0400 Subject: [PATCH 1/2] change CORS allow origin default to allow no origins Today, we disable CORS by default, but if a user simply enables CORS their instance of elasticsearch will allow cross origin requests from anywhere, as the default value for allowed origins is `*`. This changes the default to be `null` so that no origins are allowed and the user must explicitly specify the origins they wish to allow requests from. The documentation also mentions that there is a security risk in using `*` as the value. Closes #11169 --- .../http/netty/NettyHttpChannel.java | 8 +- .../http/netty/NettyHttpChannelTests.java | 350 ++++++++++++++++++ docs/reference/modules/http.asciidoc | 8 +- 3 files changed, 359 insertions(+), 7 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/http/netty/NettyHttpChannelTests.java diff --git a/core/src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java b/core/src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java index c5bcab55679..6e963261449 100644 --- a/core/src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java +++ b/core/src/main/java/org/elasticsearch/http/netty/NettyHttpChannel.java @@ -20,12 +20,10 @@ package org.elasticsearch.http.netty; import com.google.common.base.Strings; -import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.ReleasableBytesStreamOutput; import org.elasticsearch.common.lease.Releasable; -import org.elasticsearch.common.netty.NettyUtils; import org.elasticsearch.common.netty.ReleaseChannelFutureListener; import org.elasticsearch.http.HttpChannel; import org.elasticsearch.http.netty.pipelining.OrderedDownstreamChannelEvent; @@ -34,7 +32,6 @@ import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.support.RestUtils; import org.jboss.netty.buffer.ChannelBuffer; -import org.jboss.netty.buffer.ChannelBuffers; import org.jboss.netty.channel.*; import org.jboss.netty.handler.codec.http.*; @@ -100,7 +97,10 @@ public class NettyHttpChannel extends HttpChannel { String originHeader = request.header(ORIGIN); if (!Strings.isNullOrEmpty(originHeader)) { if (corsPattern == null) { - resp.headers().add(ACCESS_CONTROL_ALLOW_ORIGIN, transport.settings().get(SETTING_CORS_ALLOW_ORIGIN, "*")); + String allowedOrigins = transport.settings().get(SETTING_CORS_ALLOW_ORIGIN, null); + if (!Strings.isNullOrEmpty(allowedOrigins)) { + resp.headers().add(ACCESS_CONTROL_ALLOW_ORIGIN, allowedOrigins); + } } else { resp.headers().add(ACCESS_CONTROL_ALLOW_ORIGIN, corsPattern.matcher(originHeader).matches() ? originHeader : "null"); } diff --git a/core/src/test/java/org/elasticsearch/http/netty/NettyHttpChannelTests.java b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpChannelTests.java new file mode 100644 index 00000000000..c40f31f9acb --- /dev/null +++ b/core/src/test/java/org/elasticsearch/http/netty/NettyHttpChannelTests.java @@ -0,0 +1,350 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.http.netty; + +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.network.NetworkService; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.test.ElasticsearchTestCase; +import org.elasticsearch.test.cache.recycler.MockBigArrays; +import org.elasticsearch.test.cache.recycler.MockPageCacheRecycler; +import org.elasticsearch.threadpool.ThreadPool; +import org.jboss.netty.buffer.ChannelBuffer; +import org.jboss.netty.buffer.ChannelBuffers; +import org.jboss.netty.channel.*; +import org.jboss.netty.handler.codec.http.*; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.net.SocketAddress; +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.*; + +public class NettyHttpChannelTests extends ElasticsearchTestCase { + + private NetworkService networkService; + private ThreadPool threadPool; + private MockBigArrays bigArrays; + private NettyHttpServerTransport httpServerTransport; + + @Before + public void setup() throws Exception { + networkService = new NetworkService(Settings.EMPTY); + threadPool = new ThreadPool("test"); + MockPageCacheRecycler mockPageCacheRecycler = new MockPageCacheRecycler(Settings.EMPTY, threadPool); + bigArrays = new MockBigArrays(mockPageCacheRecycler, new NoneCircuitBreakerService()); + } + + @After + public void shutdown() throws Exception { + if (threadPool != null) { + threadPool.shutdownNow(); + } + if (httpServerTransport != null) { + httpServerTransport.close(); + } + } + + @Test + public void testCorsEnabledWithoutAllowOrigins() { + // Set up a HTTP transport with only the CORS enabled setting + Settings settings = Settings.builder() + .put(NettyHttpServerTransport.SETTING_CORS_ENABLED, true) + .build(); + httpServerTransport = new NettyHttpServerTransport(settings, networkService, bigArrays); + HttpRequest httpRequest = new TestHttpRequest(); + httpRequest.headers().add(HttpHeaders.Names.ORIGIN, "remote"); + httpRequest.headers().add(HttpHeaders.Names.USER_AGENT, "Mozilla fake"); + WriteCapturingChannel writeCapturingChannel = new WriteCapturingChannel(); + NettyHttpRequest request = new NettyHttpRequest(httpRequest, writeCapturingChannel); + + // send a response + NettyHttpChannel channel = new NettyHttpChannel(httpServerTransport, request, null, randomBoolean()); + channel.sendResponse(new TestReponse()); + + // inspect what was written + List writtenObjects = writeCapturingChannel.getWrittenObjects(); + assertThat(writtenObjects.size(), is(1)); + HttpResponse response = (HttpResponse) writtenObjects.get(0); + assertThat(response.headers().get(HttpHeaders.Names.ACCESS_CONTROL_ALLOW_ORIGIN), nullValue()); + } + + @Test + public void testCorsEnabledWithAllowOrigins() { + // create a http transport with CORS enabled and allow origin configured + Settings settings = Settings.builder() + .put(NettyHttpServerTransport.SETTING_CORS_ENABLED, true) + .put(NettyHttpServerTransport.SETTING_CORS_ALLOW_ORIGIN, "remote-host") + .build(); + httpServerTransport = new NettyHttpServerTransport(settings, networkService, bigArrays); + HttpRequest httpRequest = new TestHttpRequest(); + httpRequest.headers().add(HttpHeaders.Names.ORIGIN, "remote"); + httpRequest.headers().add(HttpHeaders.Names.USER_AGENT, "Mozilla fake"); + WriteCapturingChannel writeCapturingChannel = new WriteCapturingChannel(); + NettyHttpRequest request = new NettyHttpRequest(httpRequest, writeCapturingChannel); + + NettyHttpChannel channel = new NettyHttpChannel(httpServerTransport, request, null, randomBoolean()); + channel.sendResponse(new TestReponse()); + + // inspect what was written + List writtenObjects = writeCapturingChannel.getWrittenObjects(); + assertThat(writtenObjects.size(), is(1)); + HttpResponse response = (HttpResponse) writtenObjects.get(0); + assertThat(response.headers().get(HttpHeaders.Names.ACCESS_CONTROL_ALLOW_ORIGIN), notNullValue()); + String allowedOrigins = response.headers().get(HttpHeaders.Names.ACCESS_CONTROL_ALLOW_ORIGIN); + assertThat(allowedOrigins, is("remote-host")); + } + + private static class WriteCapturingChannel implements Channel { + + private List writtenObjects = new ArrayList<>(); + + @Override + public Integer getId() { + return null; + } + + @Override + public ChannelFactory getFactory() { + return null; + } + + @Override + public Channel getParent() { + return null; + } + + @Override + public ChannelConfig getConfig() { + return null; + } + + @Override + public ChannelPipeline getPipeline() { + return null; + } + + @Override + public boolean isOpen() { + return false; + } + + @Override + public boolean isBound() { + return false; + } + + @Override + public boolean isConnected() { + return false; + } + + @Override + public SocketAddress getLocalAddress() { + return null; + } + + @Override + public SocketAddress getRemoteAddress() { + return null; + } + + @Override + public ChannelFuture write(Object message) { + writtenObjects.add(message); + return null; + } + + @Override + public ChannelFuture write(Object message, SocketAddress remoteAddress) { + writtenObjects.add(message); + return null; + } + + @Override + public ChannelFuture bind(SocketAddress localAddress) { + return null; + } + + @Override + public ChannelFuture connect(SocketAddress remoteAddress) { + return null; + } + + @Override + public ChannelFuture disconnect() { + return null; + } + + @Override + public ChannelFuture unbind() { + return null; + } + + @Override + public ChannelFuture close() { + return null; + } + + @Override + public ChannelFuture getCloseFuture() { + return null; + } + + @Override + public int getInterestOps() { + return 0; + } + + @Override + public boolean isReadable() { + return false; + } + + @Override + public boolean isWritable() { + return false; + } + + @Override + public ChannelFuture setInterestOps(int interestOps) { + return null; + } + + @Override + public ChannelFuture setReadable(boolean readable) { + return null; + } + + @Override + public boolean getUserDefinedWritability(int index) { + return false; + } + + @Override + public void setUserDefinedWritability(int index, boolean isWritable) { + + } + + @Override + public Object getAttachment() { + return null; + } + + @Override + public void setAttachment(Object attachment) { + + } + + @Override + public int compareTo(Channel o) { + return 0; + } + + public List getWrittenObjects() { + return writtenObjects; + } + } + + private static class TestHttpRequest implements HttpRequest { + + private HttpHeaders headers = new DefaultHttpHeaders(); + + @Override + public HttpMethod getMethod() { + return null; + } + + @Override + public void setMethod(HttpMethod method) { + + } + + @Override + public String getUri() { + return ""; + } + + @Override + public void setUri(String uri) { + + } + + @Override + public HttpVersion getProtocolVersion() { + return HttpVersion.HTTP_1_1; + } + + @Override + public void setProtocolVersion(HttpVersion version) { + + } + + @Override + public HttpHeaders headers() { + return headers; + } + + @Override + public ChannelBuffer getContent() { + return ChannelBuffers.EMPTY_BUFFER; + } + + @Override + public void setContent(ChannelBuffer content) { + + } + + @Override + public boolean isChunked() { + return false; + } + + @Override + public void setChunked(boolean chunked) { + + } + } + + private static class TestReponse extends RestResponse { + + @Override + public String contentType() { + return "text"; + } + + @Override + public BytesReference content() { + return BytesArray.EMPTY; + } + + @Override + public RestStatus status() { + return RestStatus.OK; + } + } +} diff --git a/docs/reference/modules/http.asciidoc b/docs/reference/modules/http.asciidoc index 8baa8376444..95399450828 100644 --- a/docs/reference/modules/http.asciidoc +++ b/docs/reference/modules/http.asciidoc @@ -55,11 +55,13 @@ Defaults to `6`. i.e. whether a browser on another origin can do requests to Elasticsearch. Defaults to `false`. -|`http.cors.allow-origin` |Which origins to allow. Defaults to `*`, -i.e. any origin. If you prepend and append a `/` to the value, this will +|`http.cors.allow-origin` |Which origins to allow. Defaults to no origins +allowed. If you prepend and append a `/` to the value, this will be treated as a regular expression, allowing you to support HTTP and HTTPs. for example using `/https?:\/\/localhost(:[0-9]+)?/` would return the -request header appropriately in both cases. +request header appropriately in both cases. `*` is a valid value but is +considered a *secruity risk* as your elasticsearch instance is open to cross origin +requests from *anywhere*. |`http.cors.max-age` |Browsers send a "preflight" OPTIONS-request to determine CORS settings. `max-age` defines how long the result should From 8876ddf90bb48a28580e8a712c7ef9de061f61ed Mon Sep 17 00:00:00 2001 From: jaymode Date: Wed, 8 Jul 2015 15:24:14 -0400 Subject: [PATCH 2/2] fix spelling and add to migration docs --- docs/reference/migration/migrate_2_0.asciidoc | 11 +++++++++++ docs/reference/modules/http.asciidoc | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_2_0.asciidoc b/docs/reference/migration/migrate_2_0.asciidoc index 84053a82f44..1cd4f5d20be 100644 --- a/docs/reference/migration/migrate_2_0.asciidoc +++ b/docs/reference/migration/migrate_2_0.asciidoc @@ -450,6 +450,17 @@ http.cors.enabled: true http.cors.allow-origin: /https?:\/\/localhost(:[0-9]+)?/ --------------- +=== CORS allowed origins + +The CORS allowed origins setting, `http.cors.allow-origin`, no longer has a default value. Previously, the default value +was `*`, which would allow CORS requests from any origin and is considered insecure. The `http.cors.allow-origin` setting +should be specified with only the origins that should be allowed, like so: + +[source,yaml] +--------------- +http.cors.allow-origin: /https?:\/\/localhost(:[0-9]+)?/ +--------------- + === Cluster state REST api The cluster state api doesn't return the `routing_nodes` section anymore when diff --git a/docs/reference/modules/http.asciidoc b/docs/reference/modules/http.asciidoc index 95399450828..3255361bf86 100644 --- a/docs/reference/modules/http.asciidoc +++ b/docs/reference/modules/http.asciidoc @@ -60,7 +60,7 @@ allowed. If you prepend and append a `/` to the value, this will be treated as a regular expression, allowing you to support HTTP and HTTPs. for example using `/https?:\/\/localhost(:[0-9]+)?/` would return the request header appropriately in both cases. `*` is a valid value but is -considered a *secruity risk* as your elasticsearch instance is open to cross origin +considered a *security risk* as your elasticsearch instance is open to cross origin requests from *anywhere*. |`http.cors.max-age` |Browsers send a "preflight" OPTIONS-request to