From f05808d59e344ccee07ace5f26dece190c6710ec Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Fri, 15 May 2015 08:40:40 +0200 Subject: [PATCH] HttpServer: Support relative plugin paths in configuration When specifying relative paths on startup, handling plugin paths failed due to recently added security fix. This fix ensures normalization of the plugin path as well. In addition a new matcher has been added to easily check for a status code of an HTTP response likes this assertThat(response, hasStatus(OK)); Closes #10958 --- .../org/elasticsearch/http/HttpServer.java | 2 +- .../plugins/ResponseHeaderPluginTests.java | 13 ++- .../SitePluginRelativePathConfigTests.java | 90 +++++++++++++++++++ .../plugins/SitePluginTests.java | 23 +++-- .../hamcrest/ElasticsearchAssertions.java | 5 ++ .../test/hamcrest/ElasticsearchMatchers.java | 27 ++++++ 6 files changed, 139 insertions(+), 21 deletions(-) create mode 100644 src/test/java/org/elasticsearch/plugins/SitePluginRelativePathConfigTests.java diff --git a/src/main/java/org/elasticsearch/http/HttpServer.java b/src/main/java/org/elasticsearch/http/HttpServer.java index a40fc96a29d..5199ea5e422 100644 --- a/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/src/main/java/org/elasticsearch/http/HttpServer.java @@ -183,7 +183,7 @@ public class HttpServer extends AbstractLifecycleComponent { Path file = siteFile.resolve(sitePath); // return not found instead of forbidden to prevent malicious requests to find out if files exist or dont exist - if (!Files.exists(file) || Files.isHidden(file) || !file.toAbsolutePath().normalize().startsWith(siteFile.toAbsolutePath())) { + if (!Files.exists(file) || Files.isHidden(file) || !file.toAbsolutePath().normalize().startsWith(siteFile.toAbsolutePath().normalize())) { channel.sendResponse(new BytesRestResponse(NOT_FOUND)); return; } diff --git a/src/test/java/org/elasticsearch/plugins/ResponseHeaderPluginTests.java b/src/test/java/org/elasticsearch/plugins/ResponseHeaderPluginTests.java index 852c4471ab8..148f81391de 100644 --- a/src/test/java/org/elasticsearch/plugins/ResponseHeaderPluginTests.java +++ b/src/test/java/org/elasticsearch/plugins/ResponseHeaderPluginTests.java @@ -18,19 +18,18 @@ */ package org.elasticsearch.plugins; -import org.apache.http.impl.client.HttpClients; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.http.HttpServerTransport; -import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.plugins.responseheader.TestResponseHeaderPlugin; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; -import org.elasticsearch.test.rest.client.http.HttpRequestBuilder; import org.elasticsearch.test.rest.client.http.HttpResponse; -import org.elasticsearch.plugins.responseheader.TestResponseHeaderPlugin; import org.junit.Test; +import static org.elasticsearch.rest.RestStatus.OK; +import static org.elasticsearch.rest.RestStatus.UNAUTHORIZED; import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasStatus; import static org.hamcrest.Matchers.equalTo; /** @@ -52,11 +51,11 @@ public class ResponseHeaderPluginTests extends ElasticsearchIntegrationTest { public void testThatSettingHeadersWorks() throws Exception { ensureGreen(); HttpResponse response = httpClient().method("GET").path("/_protected").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.UNAUTHORIZED.getStatus())); + assertThat(response, hasStatus(UNAUTHORIZED)); assertThat(response.getHeaders().get("Secret"), equalTo("required")); HttpResponse authResponse = httpClient().method("GET").path("/_protected").addHeader("Secret", "password").execute(); - assertThat(authResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(authResponse, hasStatus(OK)); assertThat(authResponse.getHeaders().get("Secret"), equalTo("granted")); } diff --git a/src/test/java/org/elasticsearch/plugins/SitePluginRelativePathConfigTests.java b/src/test/java/org/elasticsearch/plugins/SitePluginRelativePathConfigTests.java new file mode 100644 index 00000000000..83755d7980c --- /dev/null +++ b/src/test/java/org/elasticsearch/plugins/SitePluginRelativePathConfigTests.java @@ -0,0 +1,90 @@ +/* + * 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.plugins; + +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.elasticsearch.common.io.PathUtils; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.http.HttpServerTransport; +import org.elasticsearch.test.ElasticsearchIntegrationTest; +import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; +import org.elasticsearch.test.rest.client.http.HttpRequestBuilder; +import org.elasticsearch.test.rest.client.http.HttpResponse; +import org.junit.Test; + +import java.nio.file.Path; + +import static org.apache.lucene.util.Constants.WINDOWS; +import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; +import static org.elasticsearch.rest.RestStatus.OK; +import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope.SUITE; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasStatus; + +@ClusterScope(scope = SUITE, numDataNodes = 1) +public class SitePluginRelativePathConfigTests extends ElasticsearchIntegrationTest { + + private final Path root = PathUtils.get(".").toAbsolutePath().getRoot(); + + @Override + protected Settings nodeSettings(int nodeOrdinal) { + String cwdToRoot = getRelativePath(PathUtils.get(".").toAbsolutePath()); + Path pluginDir = PathUtils.get(cwdToRoot, relativizeToRootIfNecessary(getDataPath("/org/elasticsearch/plugins")).toString()); + + Path tempDir = createTempDir(); + boolean useRelativeInMiddleOfPath = randomBoolean(); + if (useRelativeInMiddleOfPath) { + pluginDir = PathUtils.get(tempDir.toString(), getRelativePath(tempDir), pluginDir.toString()); + } + + return settingsBuilder() + .put(super.nodeSettings(nodeOrdinal)) + .put("path.plugins", pluginDir) + .put("force.http.enabled", true) + .build(); + } + + @Test + public void testThatRelativePathsDontAffectPlugins() throws Exception { + HttpResponse response = httpClient().method("GET").path("/_plugin/dummy/").execute(); + assertThat(response, hasStatus(OK)); + } + + private Path relativizeToRootIfNecessary(Path path) { + if (WINDOWS) { + return root.relativize(path); + } + return path; + } + + private String getRelativePath(Path path) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < path.getNameCount(); i++) { + sb.append(".."); + sb.append(path.getFileSystem().getSeparator()); + } + + return sb.toString(); + } + + public HttpRequestBuilder httpClient() { + CloseableHttpClient httpClient = HttpClients.createDefault(); + return new HttpRequestBuilder(httpClient).httpTransport(internalCluster().getDataNodeInstance(HttpServerTransport.class)); + } +} diff --git a/src/test/java/org/elasticsearch/plugins/SitePluginTests.java b/src/test/java/org/elasticsearch/plugins/SitePluginTests.java index 8106c6f60dd..8df880a3aa3 100644 --- a/src/test/java/org/elasticsearch/plugins/SitePluginTests.java +++ b/src/test/java/org/elasticsearch/plugins/SitePluginTests.java @@ -21,27 +21,24 @@ package org.elasticsearch.plugins; import org.apache.http.client.config.RequestConfig; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; -import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.http.HttpServerTransport; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope; import org.elasticsearch.test.rest.client.http.HttpRequestBuilder; import org.elasticsearch.test.rest.client.http.HttpResponse; import org.junit.Test; -import java.net.URISyntaxException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import java.util.Locale; import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder; +import static org.elasticsearch.rest.RestStatus.*; import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.hasStatus; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; /** * We want to test site plugins @@ -70,12 +67,12 @@ public class SitePluginTests extends ElasticsearchIntegrationTest { public void testRedirectSitePlugin() throws Exception { // We use an HTTP Client to test redirection HttpResponse response = httpClient().method("GET").path("/_plugin/dummy").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.MOVED_PERMANENTLY.getStatus())); + assertThat(response, hasStatus(MOVED_PERMANENTLY)); assertThat(response.getBody(), containsString("/_plugin/dummy/")); // We test the real URL response = httpClient().method("GET").path("/_plugin/dummy/").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response, hasStatus(OK)); assertThat(response.getBody(), containsString("Dummy Site Plugin")); } @@ -85,7 +82,7 @@ public class SitePluginTests extends ElasticsearchIntegrationTest { @Test public void testAnyPage() throws Exception { HttpResponse response = httpClient().path("/_plugin/dummy/index.html").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response, hasStatus(OK)); assertThat(response.getBody(), containsString("Dummy Site Plugin")); } @@ -108,12 +105,12 @@ public class SitePluginTests extends ElasticsearchIntegrationTest { for (String uri : notFoundUris) { HttpResponse response = httpClient().path(uri).execute(); String message = String.format(Locale.ROOT, "URI [%s] expected to be not found", uri); - assertThat(message, response.getStatusCode(), equalTo(RestStatus.NOT_FOUND.getStatus())); + assertThat(message, response, hasStatus(NOT_FOUND)); } // using relative path inside of the plugin should work HttpResponse response = httpClient().path("/_plugin/dummy/dir1/../dir1/../index.html").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response, hasStatus(OK)); assertThat(response.getBody(), containsString("Dummy Site Plugin")); } @@ -124,14 +121,14 @@ public class SitePluginTests extends ElasticsearchIntegrationTest { @Test public void testWelcomePageInSubDirs() throws Exception { HttpResponse response = httpClient().path("/_plugin/subdir/dir/").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response, hasStatus(OK)); assertThat(response.getBody(), containsString("Dummy Site Plugin (subdir)")); response = httpClient().path("/_plugin/subdir/dir_without_index/").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.FORBIDDEN.getStatus())); + assertThat(response, hasStatus(FORBIDDEN)); response = httpClient().path("/_plugin/subdir/dir_without_index/page.html").execute(); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response, hasStatus(OK)); assertThat(response.getBody(), containsString("Dummy Site Plugin (page)")); } } diff --git a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index ab59da837e4..201bc879c67 100644 --- a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -66,6 +66,7 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.suggest.Suggest; import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.rest.client.http.HttpResponse; import org.hamcrest.Matcher; import org.hamcrest.Matchers; import org.junit.Assert; @@ -490,6 +491,10 @@ public class ElasticsearchAssertions { return new ElasticsearchMatchers.SearchHitHasScoreMatcher(score); } + public static Matcher hasStatus(RestStatus restStatus) { + return new ElasticsearchMatchers.HttpResponseHasStatusMatcher(restStatus); + } + public static T assertBooleanSubQuery(Query query, Class subqueryType, int i) { assertThat(query, instanceOf(BooleanQuery.class)); BooleanQuery q = (BooleanQuery) query; diff --git a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchMatchers.java b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchMatchers.java index f49cc3bd39e..1853d291c6d 100644 --- a/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchMatchers.java +++ b/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchMatchers.java @@ -18,8 +18,11 @@ */ package org.elasticsearch.test.hamcrest; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchHit; +import org.elasticsearch.test.rest.client.http.HttpResponse; import org.hamcrest.Description; +import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; public class ElasticsearchMatchers { @@ -115,4 +118,28 @@ public class ElasticsearchMatchers { description.appendText("searchHit score should be ").appendValue(score); } } + + public static class HttpResponseHasStatusMatcher extends TypeSafeMatcher { + + private RestStatus restStatus; + + public HttpResponseHasStatusMatcher(RestStatus restStatus) { + this.restStatus = restStatus; + } + + @Override + protected boolean matchesSafely(HttpResponse response) { + return response.getStatusCode() == restStatus.getStatus(); + } + + @Override + public void describeMismatchSafely(final HttpResponse response, final Description mismatchDescription) { + mismatchDescription.appendText(" was ").appendValue(response.getStatusCode()); + } + + @Override + public void describeTo(final Description description) { + description.appendText("HTTP response status code should be ").appendValue(restStatus.getStatus()); + } + } }