Merge pull request #11890 from jaymode/cors

change CORS allow origin default to allow no origins

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");
                     }

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<Object> 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<Object> 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<Object> 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<Object> 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;
+        }
+    }
+}

docs/reference/migration/migrate_2_0.asciidoc

@@ -457,6 +457,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

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 `*`,
+|`http.cors.allow-origin` |Which origins to allow. Defaults to no origins
-i.e. any origin. If you prepend and append a `/` to the value, this will
+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 *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
 determine CORS settings. `max-age` defines how long the result should